[ovs-dev] Returned mail: Data format error

[MAILER-DAEMON]

The original message was received at Tue, 23 Aug 2016 10:28:34 +0800
from openvswitch.org [208.81.239.218]

- The following addresses had permanent fatal errors -


- Transcript of session follows -
... while talking to 142.210.86.51:
554 ... Message is too large
554 ... Service unavailable

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

[ovs-dev] [PATCH V15] Function tracer to trace all function calls

[nghosh]

From: Nirapada Ghosh 

In some circumstances, we might need to figure out where in
code, the CPU time is being spent most, so as to pinpoint
the bottleneck and thereby resolve it with proper changes.
Using '-finstrument-functions' flag, that can be achieved, and
this patch exactly does that.

There is a python file [generate_ft_report.py] with the patch,
that may be used to convert this trace output to a human readable
format with symbol names instead of address and their execution
times. This tool uses addr2line that expects the executable to
be built with -g flag.

To enable this feature, ovs needs to be configured with
"--enable-ft" command line argument [i.e. configure --enable-ft]

This feature logs the tracing output to log files,
that is set using "ovs-appctl vlog/trace " command.
"ovs-appctl vlog/trace off" turns the logging off.

The feature uses the hooks GNU-C provides when compiled with
-finstrument-functions flag, we just have to implement
them. What it means is, once you compile the code with --enable-ft
option, function calls are going to be routed to the tracing routine
anyways. In other words, even if we do disable tracing, the extra calls would
be happening though with very less CPU overhead, because the calls
would return right away. The initialization call [ constructor ] happens
even before main() is invoked, so no chance of completely disabling
tracing when configured with --enable-ft. So, unless you intend on debugging
a module in OVS, this option would better be turned off by default.

It is intended to be used for debugging purposes only. Turning it ON will
add two calls for every function call [entry and exit] and thereby two
log lines into trace log output file, please turn it on when you need to
and turn it off when done.

Currently working on adding a knob to set the min and max stack depth beyond 
which
functions won't be traced, any suggestions/comments are welcome.

Signed-off-by: Nirapada Ghosh 
---
 configure.ac|  10 +++
 lib/vlog-unixctl.man|   9 +++
 lib/vlog.c  | 139 
 utilities/automake.mk   |   8 +++
 utilities/generate_ft_report.py |  83 
 utilities/ovs-appctl.8.in   |   8 +++
 6 files changed, 257 insertions(+)
 create mode 100644 utilities/generate_ft_report.py

diff --git a/configure.ac b/configure.ac
index 2f854dd..2bcad39 100644
--- a/configure.ac
+++ b/configure.ac
@@ -28,6 +28,16 @@ AC_PROG_MKDIR_P
 AC_PROG_FGREP
 AC_PROG_EGREP
 
+AC_ARG_ENABLE([ft],
+  [AC_HELP_STRING([--enable-ft], [Turn on function tracing])],
+  [case "${enableval}" in
+(yes) ft=true ;;
+(no)  ft=false ;;
+(*) AC_MSG_ERROR([bad value ${enableval} for --enable-ft]) ;;
+  esac],
+  [ft=false])
+AM_CONDITIONAL([ENABLE_FT], [test x$ft = xtrue])
+
 AC_ARG_VAR([PERL], [path to Perl interpreter])
 AC_PATH_PROG([PERL], perl, no)
 if test "$PERL" = no; then
diff --git a/lib/vlog-unixctl.man b/lib/vlog-unixctl.man
index 7372a7e..71d0f8f 100644
--- a/lib/vlog-unixctl.man
+++ b/lib/vlog-unixctl.man
@@ -82,3 +82,12 @@ the keyword \fBany\fR disables rate limits for every log 
module.
 The \fBvlog/enable\-rate\-limit\fR command, whose syntax is the same
 as \fBvlog/disable\-rate\-limit\fR, can be used to re-enable a rate
 limit that was previously disabled.
+.IP
+This command is also used to enable/disable function-tracing, availble
+only when configured with --enable-ft and only with GNUC.
+.IP "\fBvlog/trace\fR [\fIfilename\fR]"
+Sets function tracing on or off. If "off" is passed, it
+turns off tracing for the module in question, Otherwise,
+\fIfilename\fR is the name of the trace log file and tracing will
+be turned on with this command automatically.
+.IP
diff --git a/lib/vlog.c b/lib/vlog.c
index 37b..313ffaa 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -46,6 +46,29 @@
 
 VLOG_DEFINE_THIS_MODULE(vlog);
 
+#if defined(ENABLE_FT) && defined(__GNUC__)
+/* File pointer for logging trace output. */
+static FILE *function_tracing_fp;
+/* Global flag holding current state of function-tracing-enabled or not. */
+static bool function_tracing_enabled = false;
+
+/* Prototypes for the functions declared/used in this file. */
+static void vlog_unixctl_set_fn_tracing(struct unixctl_conn *conn,
+int argc,
+const char *argv[],
+void *aux OVS_UNUSED);
+static char * vlog_set_fn_tracing_log(const char *s_);
+void __attribute__ ((constructor,no_instrument_function)) fn_trace_begin(void);
+void __attribute__ ((destructor,no_instrument_function)) fn_trace_end(void);
+static void __attribute__ ((no_instrument_function)) trace_func_call(
+ const char * direction,
+ void *func, void * caller);
+void __attribute__ 

[ovs-dev] [PATCH V15] Function tracer to trace all function calls

[nghosh]

From: Nirapada Ghosh 

In some circumstances, we might need to figure out where in
code, the CPU time is being spent most, so as to pinpoint
the bottleneck and thereby resolve it with proper changes.
Using '-finstrument-functions' flag, that can be achieved, and
this patch exactly does that.

There is a python file [generate_ft_report.py] with the patch,
that may be used to convert this trace output to a human readable
format with symbol names instead of address and their execution
times. This tool uses addr2line that expects the executable to
be built with -g flag.

To enable this feature, ovs needs to be configured with
"--enable-ft" command line argument [i.e. configure --enable-ft]

This feature logs the tracing output to log files,
that is set using "ovs-appctl vlog/trace " command.
"ovs-appctl vlog/trace off" turns the logging off.

The feature uses the hooks GNU-C provides when compiled with
-finstrument-functions flag, we just have to implement
them. What it means is, once you compile the code with --enable-ft
option, function calls are going to be routed to the tracing routine
anyways. In other words, even if we do disable tracing, the extra calls would
be happening though with very less CPU overhead, because the calls
would return right away. The initialization call [ constructor ] happens
even before main() is invoked, so no chance of completely disabling
tracing when configured with --enable-ft. So, unless you intend on debugging
a module in OVS, this option would better be turned off by default.

It is intended to be used for debugging purposes only. Turning it ON will
add two calls for every function call [entry and exit] and thereby two
log lines into trace log output file, please turn it on when you need to
and turn it off when done.

Currently working on adding a knob to set the min and max stack depth beyond 
which
functions won't be traced, any suggestions/comments are welcome.

Signed-off-by: Nirapada Ghosh 
---
 configure.ac|  10 +++
 lib/vlog-unixctl.man|   9 +++
 lib/vlog.c  | 139 
 utilities/automake.mk   |   8 +++
 utilities/generate_ft_report.py |  83 
 utilities/ovs-appctl.8.in   |   8 +++
 6 files changed, 257 insertions(+)
 create mode 100644 utilities/generate_ft_report.py

diff --git a/configure.ac b/configure.ac
index 2f854dd..2bcad39 100644
--- a/configure.ac
+++ b/configure.ac
@@ -28,6 +28,16 @@ AC_PROG_MKDIR_P
 AC_PROG_FGREP
 AC_PROG_EGREP
 
+AC_ARG_ENABLE([ft],
+  [AC_HELP_STRING([--enable-ft], [Turn on function tracing])],
+  [case "${enableval}" in
+(yes) ft=true ;;
+(no)  ft=false ;;
+(*) AC_MSG_ERROR([bad value ${enableval} for --enable-ft]) ;;
+  esac],
+  [ft=false])
+AM_CONDITIONAL([ENABLE_FT], [test x$ft = xtrue])
+
 AC_ARG_VAR([PERL], [path to Perl interpreter])
 AC_PATH_PROG([PERL], perl, no)
 if test "$PERL" = no; then
diff --git a/lib/vlog-unixctl.man b/lib/vlog-unixctl.man
index 7372a7e..71d0f8f 100644
--- a/lib/vlog-unixctl.man
+++ b/lib/vlog-unixctl.man
@@ -82,3 +82,12 @@ the keyword \fBany\fR disables rate limits for every log 
module.
 The \fBvlog/enable\-rate\-limit\fR command, whose syntax is the same
 as \fBvlog/disable\-rate\-limit\fR, can be used to re-enable a rate
 limit that was previously disabled.
+.IP
+This command is also used to enable/disable function-tracing, availble
+only when configured with --enable-ft and only with GNUC.
+.IP "\fBvlog/trace\fR [\fIfilename\fR]"
+Sets function tracing on or off. If "off" is passed, it
+turns off tracing for the module in question, Otherwise,
+\fIfilename\fR is the name of the trace log file and tracing will
+be turned on with this command automatically.
+.IP
diff --git a/lib/vlog.c b/lib/vlog.c
index 37b..313ffaa 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -46,6 +46,29 @@
 
 VLOG_DEFINE_THIS_MODULE(vlog);
 
+#if defined(ENABLE_FT) && defined(__GNUC__)
+/* File pointer for logging trace output. */
+static FILE *function_tracing_fp;
+/* Global flag holding current state of function-tracing-enabled or not. */
+static bool function_tracing_enabled = false;
+
+/* Prototypes for the functions declared/used in this file. */
+static void vlog_unixctl_set_fn_tracing(struct unixctl_conn *conn,
+int argc,
+const char *argv[],
+void *aux OVS_UNUSED);
+static char * vlog_set_fn_tracing_log(const char *s_);
+void __attribute__ ((constructor,no_instrument_function)) fn_trace_begin(void);
+void __attribute__ ((destructor,no_instrument_function)) fn_trace_end(void);
+static void __attribute__ ((no_instrument_function)) trace_func_call(
+ const char * direction,
+ void *func, void * caller);
+void __attribute__ 

[ovs-dev] [PATCH v1] ovn: ensure datapath removal is proper

[Flavio Fernandes]

Adding a unit test in ovn.at, to exercise the cleanup of
OF rules related to a logical datapath, when a logical
switch is removed.

Reported-by: Guru Shetty 
Reported-at: http://openvswitch.org/pipermail/discuss/2016-August/022478.html
Signed-off-by: Flavio Fernandes 
---
NOTE: This test is expected to fail at this time, as the bug is not yet fixed.

 tests/ovn.at | 58 ++
 1 file changed, 58 insertions(+)

diff --git a/tests/ovn.at b/tests/ovn.at
index 216bb07..604cdcb 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -4319,6 +4319,64 @@ OVN_CLEANUP([hv1])
 
 AT_CLEANUP
 
+# 1 hypervisor, 1 port
+# make sure that the OF rules created to support a datapath are added/cleared
+# when logical switch is created and removed.
+AT_SETUP([ovn -- datapath rules added/removed])
+AT_KEYWORDS([ovn datapath cleanup])
+ovn_start
+
+net_add n1
+sim_add hv1
+as hv1 ovs-vsctl add-br br-phys
+as hv1 ovn_attach n1 br-phys 192.168.0.1
+
+# This shell function checks if OF rules in br-int have clauses
+# related to OVN datapaths. The caller determines if it should find
+# a match in the output, or not.
+test_datapath_in_of_rules() {
+local expect_datapath=$1 stage_info=$2
+echo "-- ovn-nbctl show ${stage_info} --"
+ovn-nbctl show
+echo "-- ovn-sbctl show ${stage_info} --"
+ovn-sbctl show
+echo "-- OF rules ${stage_info} --"
+AT_CHECK([ovs-ofctl dump-flows br-int], [0], [stdout])
+# if there is a datapath mentioned in the output, check for the
+# magic keyword that represents one, based on the exit status of
+# a quiet grep
+if test $expect_datapath != 0; then
+   AT_CHECK([grep --quiet -i 'metadata=' stdout], [0], [ignore-nolog])
+else
+   AT_CHECK([grep --quiet -i 'metadata=' stdout], [1], [ignore-nolog])
+fi
+}
+
+test_datapath_in_of_rules 0 "before ls+port create"
+
+ovn-nbctl ls-add ls1
+ovn-nbctl lsp-add ls1 lp1
+ovn-nbctl lsp-set-addresses lp1 unknown
+
+as hv1 ovs-vsctl add-port br-int vif1 -- set Interface vif1 
external-ids:iface-id=lp1
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp1` = xup])
+
+test_datapath_in_of_rules 1 "after port is bound"
+
+as hv1 ovs-vsctl del-port br-int vif1
+OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp1` = xdown])
+
+ovn-nbctl lsp-set-addresses lp1
+ovn-nbctl lsp-del lp1
+ovn-nbctl ls-del ls1
+
+# NOTE: if a known bug is present, THIS WILL BREAK HERE
+test_datapath_in_of_rules 0 "after ls+port removal"
+
+OVN_CLEANUP([hv1])
+
+AT_CLEANUP
+
 AT_SETUP([ovn -- nd_na ])
 AT_KEYWORDS([ovn-nd_na])
 AT_SKIP_IF([test $HAVE_PYTHON = no])
-- 
2.7.4

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

[ovs-dev] [PATCH v2 15/15] ofproto: Support packet_outs in bundles.

[Jarno Rajahalme]

Add support for OFPT_PACKET_OUT messages in bundles.

While ovs-ofctl already has a packet-out command, we did not have a
string parser for it, as the parsing was done directly from command
line arguments.

This patch adds the string parser for packet-out messages, and adds a
new ofctl/packet-out ovs-appctl command that can be used when
ovs-ofctl is used as a flow monitor.

The new packet-out parser is further supported with the ovs-ofctl
bundle command, which allows bundles to mix flow mods, group mods and
packet-out messages.  Also the packet-outs in bundles are only
executed if the whole bundle is successful.  A failing packet-out
translation may also make the whole bundle to fail.

Signed-off-by: Jarno Rajahalme 
---
 NEWS|   3 +-
 include/openvswitch/ofp-parse.h |   5 ++
 include/openvswitch/ofp-util.h  |   1 +
 lib/ofp-parse.c | 108 
 lib/ofp-util.c  |   7 ++-
 ofproto/bundles.h   |   7 ++-
 ofproto/ofproto.c   |  82 +---
 tests/ofproto.at| 134 ++--
 utilities/ovs-ofctl.8.in|  46 --
 utilities/ovs-ofctl.c   |  55 +
 10 files changed, 412 insertions(+), 36 deletions(-)

diff --git a/NEWS b/NEWS
index 4c4785d..e9b7bd2 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,7 @@
 Post-v2.6.0
 -
-
+   - OpenFlow:
+ * OFPT_PACKET_OUT messages are now supported in bundles.
 
 v2.6.0 - xx xxx 
 -
diff --git a/include/openvswitch/ofp-parse.h b/include/openvswitch/ofp-parse.h
index df60b18..e68e57b 100644
--- a/include/openvswitch/ofp-parse.h
+++ b/include/openvswitch/ofp-parse.h
@@ -28,6 +28,7 @@
 struct flow;
 struct ofpbuf;
 struct ofputil_flow_mod;
+struct ofputil_packet_out;
 struct ofputil_flow_monitor_request;
 struct ofputil_flow_stats_request;
 struct ofputil_group_mod;
@@ -47,6 +48,10 @@ char *parse_ofp_flow_mod_str(struct ofputil_flow_mod *, 
const char *string,
  enum ofputil_protocol *usable_protocols)
 OVS_WARN_UNUSED_RESULT;
 
+char *parse_ofp_packet_out_str(struct ofputil_packet_out *po, const char *str_,
+   enum ofputil_protocol *usable_protocols)
+OVS_WARN_UNUSED_RESULT;
+
 char *parse_ofp_table_mod(struct ofputil_table_mod *,
   const char *table_id, const char *flow_miss_handling,
   uint32_t *usable_versions)
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index 450e739..9283e13 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -1343,6 +1343,7 @@ struct ofputil_bundle_msg {
 union {
 struct ofputil_flow_mod fm;
 struct ofputil_group_mod gm;
+struct ofputil_packet_out po;
 };
 };
 
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index d3ef140..7fccb48 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -22,6 +22,7 @@
 #include 
 
 #include "byte-order.h"
+#include "dp-packet.h"
 #include "learn.h"
 #include "multipath.h"
 #include "netdev.h"
@@ -550,6 +551,106 @@ parse_ofp_str(struct ofputil_flow_mod *fm, int command, 
const char *str_,
 return error;
 }
 
+/* Parse a string representation of a OFPT_PACKET_OUT to '*po'.  If successful,
+ * both 'po->ofpacts' and 'po->packet' must be free()d by the caller. */
+static char * OVS_WARN_UNUSED_RESULT
+parse_ofp_packet_out_str__(struct ofputil_packet_out *po, char *string,
+   enum ofputil_protocol *usable_protocols)
+{
+enum ofputil_protocol action_usable_protocols;
+uint64_t stub[256 / 8];
+struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
+struct dp_packet *packet = NULL;
+char *act_str = NULL;
+char *name, *value;
+char *error = NULL;
+
+*usable_protocols = OFPUTIL_P_ANY;
+
+*po = (struct ofputil_packet_out) {
+.buffer_id = UINT32_MAX,
+.in_port = OFPP_CONTROLLER,
+};
+
+act_str = extract_actions(string);
+
+while (ofputil_parse_key_value(, , )) {
+if (!*value) {
+error = xasprintf("field %s missing value", name);
+}
+
+if (!strcmp(name, "in_port")) {
+if (!ofputil_port_from_string(value, >in_port)) {
+error = xasprintf("%s is not a valid OpenFlow port", value);
+}
+if (po->in_port > OFPP_MAX && po->in_port != OFPP_LOCAL
+&& po->in_port != OFPP_NONE
+&& po->in_port != OFPP_CONTROLLER) {
+error = xasprintf(
+  "%s is not a valid OpenFlow port for PACKET_OUT",
+  value);
+}
+} else if (!strcmp(name, "packet")) {
+const char *error_msg = eth_from_hex(value, );
+if (error_msg) {
+error = xasprintf("%s: %s", name, 

[ovs-dev] [PATCH v2 09/15] ofproto-dpif-xlate: Expose xlate cache.

[Jarno Rajahalme]

Later patches will need to create xlate cache entries from different
modules.  This patch refactors the xlate cache code in preparation
without any functional changes, so that the changes are clearly
visible in the following patches.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif-xlate.c | 269 ---
 ofproto/ofproto-dpif-xlate.h |  80 -
 2 files changed, 180 insertions(+), 169 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 893c033..2e89845 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -448,83 +448,6 @@ struct skb_priority_to_dscp {
 uint8_t dscp;   /* DSCP bits to mark outgoing traffic with. */
 };
 
-enum xc_type {
-XC_RULE,
-XC_BOND,
-XC_NETDEV,
-XC_NETFLOW,
-XC_MIRROR,
-XC_LEARN,
-XC_NORMAL,
-XC_FIN_TIMEOUT,
-XC_GROUP,
-XC_TNL_NEIGH,
-};
-
-/* xlate_cache entries hold enough information to perform the side effects of
- * xlate_actions() for a rule, without needing to perform rule translation
- * from scratch. The primary usage of these is to submit statistics to objects
- * that a flow relates to, although they may be used for other effects as well
- * (for instance, refreshing hard timeouts for learned flows). */
-struct xc_entry {
-enum xc_type type;
-union {
-struct rule_dpif *rule;
-struct {
-struct netdev *tx;
-struct netdev *rx;
-struct bfd *bfd;
-} dev;
-struct {
-struct netflow *netflow;
-struct flow *flow;
-ofp_port_t iface;
-} nf;
-struct {
-struct mbridge *mbridge;
-mirror_mask_t mirrors;
-} mirror;
-struct {
-struct bond *bond;
-struct flow *flow;
-uint16_t vid;
-} bond;
-struct {
-struct ofproto_dpif *ofproto;
-struct ofputil_flow_mod *fm;
-struct ofpbuf *ofpacts;
-} learn;
-struct {
-struct ofproto_dpif *ofproto;
-struct flow *flow;
-int vlan;
-} normal;
-struct {
-struct rule_dpif *rule;
-uint16_t idle;
-uint16_t hard;
-} fin;
-struct {
-struct group_dpif *group;
-struct ofputil_bucket *bucket;
-} group;
-struct {
-char br_name[IFNAMSIZ];
-struct in6_addr d_ipv6;
-} tnl_neigh_cache;
-} u;
-};
-
-#define XC_ENTRY_FOR_EACH(ENTRY, ENTRIES, XCACHE)   \
-ENTRIES = XCACHE->entries;  \
-for (ENTRY = ofpbuf_try_pull(, sizeof *ENTRY);  \
- ENTRY; \
- ENTRY = ofpbuf_try_pull(, sizeof *ENTRY))
-
-struct xlate_cache {
-struct ofpbuf entries;
-};
-
 /* Xlate config contains hash maps of all bridges, bundles and ports.
  * Xcfgp contains the pointer to the current xlate configuration.
  * When the main thread needs to change the configuration, it copies xcfgp to
@@ -578,8 +501,6 @@ static size_t count_skb_priorities(const struct xport *);
 static bool dscp_from_skb_priority(const struct xport *, uint32_t skb_priority,
uint8_t *dscp);
 
-static struct xc_entry *xlate_cache_add_entry(struct xlate_cache *xc,
-  enum xc_type type);
 static void xlate_xbridge_init(struct xlate_cfg *, struct xbridge *);
 static void xlate_xbundle_init(struct xlate_cfg *, struct xbundle *);
 static void xlate_xport_init(struct xlate_cfg *, struct xport *);
@@ -5776,7 +5697,7 @@ xlate_cache_new(void)
 return xcache;
 }
 
-static struct xc_entry *
+struct xc_entry *
 xlate_cache_add_entry(struct xlate_cache *xcache, enum xc_type type)
 {
 struct xc_entry *entry;
@@ -5825,61 +5746,68 @@ xlate_cache_normal(struct ofproto_dpif *ofproto, struct 
flow *flow, int vlan)
 
 /* Push stats and perform side effects of flow translation. */
 void
-xlate_push_stats(struct xlate_cache *xcache,
- const struct dpif_flow_stats *stats)
+xlate_push_stats_entry(struct xc_entry *entry,
+   const struct dpif_flow_stats *stats)
 {
-struct xc_entry *entry;
-struct ofpbuf entries = xcache->entries;
 struct eth_addr dmac;
 
+switch (entry->type) {
+case XC_RULE:
+rule_dpif_credit_stats(entry->u.rule, stats);
+break;
+case XC_BOND:
+bond_account(entry->u.bond.bond, entry->u.bond.flow,
+ entry->u.bond.vid, stats->n_bytes);
+break;
+case XC_NETDEV:
+xlate_cache_netdev(entry, stats);
+break;
+case XC_NETFLOW:
+netflow_flow_update(entry->u.nf.netflow, entry->u.nf.flow,
+entry->u.nf.iface, stats);
+break;
+case 

[ovs-dev] [PATCH v2 08/15] connmgr: Make connmgr_wants_packet_in_on_miss() lock-free.

[Jarno Rajahalme]

Make connmgr_wants_packet_in_on_miss() use an atomic int instead of a
list traversal taking the 'ofproto_mutex'.  This allows
connmgr_wants_packet_in_on_miss() to be called also when
'ofproto_mutex' is already held, and makes it faster, too.

Remove unused ofproto_dpif_wants_packet_in_on_miss().

Signed-off-by: Jarno Rajahalme 
---
 ofproto/connmgr.c  | 59 ++
 ofproto/ofproto-dpif.c | 12 --
 ofproto/ofproto-dpif.h |  1 -
 3 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index d2bedad..d8f4abb 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -32,6 +32,7 @@
 #include "openvswitch/ofpbuf.h"
 #include "openvswitch/vconn.h"
 #include "openvswitch/vlog.h"
+#include "ovs-atomic.h"
 #include "pinsched.h"
 #include "poll-loop.h"
 #include "rconn.h"
@@ -210,6 +211,8 @@ struct connmgr {
 struct sockaddr_in *extra_in_band_remotes;
 size_t n_extra_remotes;
 int in_band_queue;
+
+ATOMIC(int) want_packet_in_on_miss;
 };
 
 static void update_in_band_remotes(struct connmgr *);
@@ -249,6 +252,8 @@ connmgr_create(struct ofproto *ofproto,
 mgr->n_extra_remotes = 0;
 mgr->in_band_queue = -1;
 
+atomic_init(>want_packet_in_on_miss, 0);
+
 return mgr;
 }
 
@@ -1200,6 +1205,32 @@ ofconn_get_target(const struct ofconn *ofconn)
 return rconn_get_target(ofconn->rconn);
 }
 
+/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the
+ * packet rather than to send the packet to the controller.
+ *
+ * This function maintains the count of pre-OpenFlow1.3 with controller_id 0,
+ * as we assume these are the controllers that should receive "table-miss"
+ * notifications.
+ *
+ * This function must be called with 'add' = 'true' whenever a controller is
+ * added, and with 'add' = 'false' whenever a controller is removed.
+ */
+static void
+update_want_packet_in_on_miss(const struct ofconn *ofconn, bool add)
+{
+if (ofconn->controller_id == 0) {
+enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
+
+if (protocol == OFPUTIL_P_NONE ||
+ofputil_protocol_to_ofp_version(protocol) < OFP13_VERSION) {
+int count;
+atomic_read_relaxed(>connmgr->want_packet_in_on_miss,
+);
+atomic_store_relaxed(>connmgr->want_packet_in_on_miss,
+ count + (add ? 1 : -1));
+}
+}
+}
 static struct ofconn *
 ofconn_create(struct connmgr *mgr, struct rconn *rconn, enum ofconn_type type,
   bool enable_async_msgs)
@@ -1220,6 +1251,8 @@ ofconn_create(struct connmgr *mgr, struct rconn *rconn, 
enum ofconn_type type,
 
 ofconn_flush(ofconn);
 
+update_want_packet_in_on_miss(ofconn, true);
+
 return ofconn;
 }
 
@@ -1280,6 +1313,8 @@ ofconn_destroy(struct ofconn *ofconn)
 {
 ofconn_flush(ofconn);
 
+update_want_packet_in_on_miss(ofconn, false);
+
 if (ofconn->type == OFCONN_PRIMARY) {
 hmap_remove(>connmgr->controllers, >hmap_node);
 }
@@ -1470,28 +1505,14 @@ ofconn_receives_async_msg(const struct ofconn *ofconn,
  * which connected using an OpenFlow version earlier than OpenFlow1.3.
  *
  * False otherwise.
- *
- * This logic assumes that "table-miss" packet_in messages
- * are always sent to controller_id 0. */
+ */
 bool
-connmgr_wants_packet_in_on_miss(struct connmgr *mgr) 
OVS_EXCLUDED(ofproto_mutex)
+connmgr_wants_packet_in_on_miss(struct connmgr *mgr)
 {
-struct ofconn *ofconn;
-
-ovs_mutex_lock(_mutex);
-LIST_FOR_EACH (ofconn, node, >all_conns) {
-enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
+int count;
 
-if (ofconn->controller_id == 0 &&
-(protocol == OFPUTIL_P_NONE ||
- ofputil_protocol_to_ofp_version(protocol) < OFP13_VERSION)) {
-ovs_mutex_unlock(_mutex);
-return true;
-}
-}
-ovs_mutex_unlock(_mutex);
-
-return false;
+atomic_read_relaxed(>want_packet_in_on_miss, );
+return count > 0;
 }
 
 /* Returns a human-readable name for an OpenFlow connection between 'mgr' and
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 289e7d6..d928f01 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -379,18 +379,6 @@ ofproto_dpif_send_async_msg(struct ofproto_dpif *ofproto,
 /* Wakes up main thread for packet-in I/O. */
 seq_change(ofproto->ams_seq);
 }
-
-/* The default "table-miss" behaviour for OpenFlow1.3+ is to drop the
- * packet rather than to send the packet to the controller.
- *
- * This function returns false to indicate that a packet_in message
- * for a "table-miss" should be sent to at least one controller.
- * False otherwise. */
-bool
-ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *ofproto)
-{
-return connmgr_wants_packet_in_on_miss(ofproto->up.connmgr);
-}
 
 /* Factory functions. */
 
diff --git 

[ovs-dev] [PATCH v2 12/15] ofproto-dpif-xlate: Allow translating without side-effects.

[Jarno Rajahalme]

Extend 'may_learn' attribute to also control the treatment of
FIN_TIMEOUT action and asynchronous messages (packet ins,
continuations), so that when 'may_learn' is 'false' and
'resubmit_stats' is 'NULL', no OpenFlow-visible side effects are
generated by the translation.

Correspondingly, add support for one-time asynchronous messages to
xlate cache, so that all side-effects of the translation may be
executed at a later stage.  This will be useful for bundle commits.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif-xlate.c | 53 +++-
 ofproto/ofproto-dpif-xlate.h | 12 +++---
 2 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2683464..83fdff7 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3595,6 +3595,10 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
 return;
 }
 
+if (!ctx->xin->may_learn && !ctx->xin->xcache) {
+return;
+}
+
 packet = dp_packet_clone(ctx->xin->packet);
 packet_batch_init_packet(, packet);
 odp_execute_actions(NULL, , false,
@@ -3633,13 +3637,26 @@ execute_controller_action(struct xlate_ctx *ctx, int 
len,
 };
 flow_get_metadata(>xin->flow, >pin.up.public.flow_metadata);
 
-ofproto_dpif_send_async_msg(ctx->xbridge->ofproto, am);
-dp_packet_delete(packet);
+/* Async messages are only sent once, so if we send one now, no
+ * xlate cache entry is created.  */
+if (ctx->xin->may_learn) {
+ofproto_dpif_send_async_msg(ctx->xbridge->ofproto, am);
+} else /* xcache */ {
+struct xc_entry *entry;
+
+entry = xlate_cache_add_entry(ctx->xin->xcache, XC_CONTROLLER);
+entry->u.controller.ofproto = ctx->xbridge->ofproto;
+entry->u.controller.am = am;
+}
 }
 
 static void
 emit_continuation(struct xlate_ctx *ctx, const struct frozen_state *state)
 {
+if (!ctx->xin->may_learn && !ctx->xin->xcache) {
+return;
+}
+
 struct ofproto_async_msg *am = xmalloc(sizeof *am);
 *am = (struct ofproto_async_msg) {
 .controller_id = ctx->pause->controller_id,
@@ -3671,7 +3688,18 @@ emit_continuation(struct xlate_ctx *ctx, const struct 
frozen_state *state)
 },
 };
 flow_get_metadata(>xin->flow, >pin.up.public.flow_metadata);
-ofproto_dpif_send_async_msg(ctx->xbridge->ofproto, am);
+
+/* Async messages are only sent once, so if we send one now, no
+ * xlate cache entry is created.  */
+if (ctx->xin->may_learn) {
+ofproto_dpif_send_async_msg(ctx->xbridge->ofproto, am);
+} else /* xcache */ {
+struct xc_entry *entry;
+
+entry = xlate_cache_add_entry(ctx->xin->xcache, XC_CONTROLLER);
+entry->u.controller.ofproto = ctx->xbridge->ofproto;
+entry->u.controller.am = am;
+}
 }
 
 static void
@@ -4121,8 +4149,10 @@ xlate_fin_timeout(struct xlate_ctx *ctx,
   const struct ofpact_fin_timeout *oft)
 {
 if (ctx->rule) {
-xlate_fin_timeout__(ctx->rule, ctx->xin->tcp_flags,
-oft->fin_idle_timeout, oft->fin_hard_timeout);
+if (ctx->xin->may_learn) {
+xlate_fin_timeout__(ctx->rule, ctx->xin->tcp_flags,
+oft->fin_idle_timeout, oft->fin_hard_timeout);
+}
 if (ctx->xin->xcache) {
 struct xc_entry *entry;
 
@@ -5810,6 +5840,13 @@ xlate_push_stats_entry(struct xc_entry *entry,
 tnl_neigh_lookup(entry->u.tnl_neigh_cache.br_name,
  >u.tnl_neigh_cache.d_ipv6, );
 break;
+case XC_CONTROLLER:
+if (entry->u.controller.am) {
+ofproto_dpif_send_async_msg(entry->u.controller.ofproto,
+entry->u.controller.am);
+entry->u.controller.am = NULL; /* One time only. */
+}
+break;
 default:
 OVS_NOT_REACHED();
 }
@@ -5890,6 +5927,12 @@ xlate_cache_clear_entry(struct xc_entry *entry)
 break;
 case XC_TNL_NEIGH:
 break;
+case XC_CONTROLLER:
+if (entry->u.controller.am) {
+ofproto_async_msg_free(entry->u.controller.am);
+entry->u.controller.am = NULL;
+}
+break;
 default:
 OVS_NOT_REACHED();
 }
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 7faa199..14d81f6 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -43,11 +43,12 @@ enum xc_type {
 XC_NETDEV,
 XC_NETFLOW,
 XC_MIRROR,
-XC_LEARN,
+XC_LEARN,/* Calls back to ofproto. */
 XC_NORMAL,
-XC_FIN_TIMEOUT,
+XC_FIN_TIMEOUT,  /* Calls back to ofproto. */
 XC_GROUP,
 XC_TNL_NEIGH,
+XC_CONTROLLER,
 };
 
 /* xlate_cache entries hold enough information to perform the side effects of
@@ -104,6 +105,10 @@ struct xc_entry {
 

[ovs-dev] [PATCH v2 14/15] ofproto: Refactor packet_out handling.

[Jarno Rajahalme]

Refactor handle_packet_out() to prepare for bundle support for packet
outs in a later patch.

Two new callbacks are introduced in ofproto-provider class:
->packet_xlate() and ->packet_execute().  ->packet_xlate() translates
the packet using the flow and actions provided by the caller, but
defers all OpenFlow-visible side-effects (stats, learn actions, actual
packet output, etc.) to be explicitly executed with the
->packet_execute() call.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif-upcall.c |  11 +-
 ofproto/ofproto-dpif-xlate.c  |  45 
 ofproto/ofproto-dpif-xlate.h  |   3 +-
 ofproto/ofproto-dpif.c| 136 
 ofproto/ofproto-dpif.h|  15 +--
 ofproto/ofproto-provider.h|  90 
 ofproto/ofproto.c | 233 +-
 7 files changed, 389 insertions(+), 144 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 042a50a..ad891be 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1058,7 +1058,9 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
 stats.used = time_msec();
 stats.tcp_flags = ntohs(upcall->flow->tcp_flags);
 
-xlate_in_init(, upcall->ofproto, upcall->flow, upcall->in_port, NULL,
+xlate_in_init(, upcall->ofproto,
+  ofproto_dpif_get_tables_version(upcall->ofproto),
+  upcall->flow, upcall->in_port, NULL,
   stats.tcp_flags, upcall->packet, wc, odp_actions);
 
 if (upcall->type == DPIF_UC_MISS) {
@@ -1849,7 +1851,8 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key 
*ukey,
 ukey->xcache = xlate_cache_new();
 }
 
-xlate_in_init(, ofproto, , ofp_in_port, NULL, push.tcp_flags,
+xlate_in_init(, ofproto, ofproto_dpif_get_tables_version(ofproto),
+  , ofp_in_port, NULL, push.tcp_flags,
   NULL, need_revalidate ?  : NULL, odp_actions);
 if (push.n_packets) {
 xin.resubmit_stats = 
@@ -2025,7 +2028,9 @@ push_dp_ops(struct udpif *udpif, struct ukey_op *ops, 
size_t n_ops)
 if (!error) {
 struct xlate_in xin;
 
-xlate_in_init(, ofproto, , ofp_in_port, NULL,
+xlate_in_init(, ofproto,
+  ofproto_dpif_get_tables_version(ofproto),
+  , ofp_in_port, NULL,
   push->tcp_flags, NULL, NULL, NULL);
 xin.resubmit_stats = push->n_packets ? push : NULL;
 xin.may_learn = push->n_packets > 0;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 83fdff7..13dab4a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -169,9 +169,6 @@ struct xlate_ctx {
 
 const struct xbridge *xbridge;
 
-/* Flow tables version at the beginning of the translation. */
-ovs_version_t tables_version;
-
 /* Flow at the last commit. */
 struct flow base_flow;
 
@@ -1431,7 +1428,7 @@ group_is_alive(const struct xlate_ctx *ctx, uint32_t 
group_id, int depth)
 struct group_dpif *group;
 
 group = group_dpif_lookup(ctx->xbridge->ofproto, group_id,
-  ctx->tables_version, false);
+  ctx->xin->tables_version, false);
 if (group) {
 return group_first_live_bucket(ctx, group, depth) != NULL;
 }
@@ -2786,10 +2783,12 @@ compose_table_xlate(struct xlate_ctx *ctx, const struct 
xport *out_dev,
 output.port = OFPP_TABLE;
 output.max_len = 0;
 
-return ofproto_dpif_execute_actions__(xbridge->ofproto, , NULL,
-  , sizeof output,
-  ctx->indentation, ctx->depth,
-  ctx->resubmits, packet);
+/* OUTPUT to a real port can not refer to versionable lookups (flow tables,
+ * groups), so we do not need to worry about the version to use here. */
+return ofproto_dpif_execute_actions__(xbridge->ofproto, OVS_VERSION_MAX,
+  , NULL, ,
+  sizeof output, ctx->indentation,
+  ctx->depth, ctx->resubmits, packet);
 }
 
 static void
@@ -2975,7 +2974,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t 
ofp_port,
 struct flow old_flow = ctx->xin->flow;
 bool old_conntrack = ctx->conntracked;
 bool old_was_mpls = ctx->was_mpls;
-ovs_version_t old_version = ctx->tables_version;
+ovs_version_t old_version = ctx->xin->tables_version;
 struct ofpbuf old_stack = ctx->stack;
 union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)];
 struct ofpbuf old_action_set = ctx->action_set;
@@ -2993,7 +2992,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t 
ofp_port,
 

[ovs-dev] [PATCH v2 10/15] ofproto-dpif-xlate: Add xlate cache type XC_TABLE.

[Jarno Rajahalme]

Xlate cache entry type XC_TABLE is required for the table stats
(number of misses and matches) to be correctly attributed.

It appears that table stats have been off ever since xlate cache was
introduced.  This was now revealed by a PACKET_OUT unit test case that
checks for table stats explicitly.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif-xlate.c | 16 +---
 ofproto/ofproto-dpif-xlate.h |  6 ++
 ofproto/ofproto-dpif.c   | 34 +-
 ofproto/ofproto-dpif.h   |  8 +++-
 4 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2e89845..6be78e6 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3275,8 +3275,8 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t 
in_port, uint8_t table_id,
>xin->flow, ctx->wc,
ctx->xin->resubmit_stats,
>table_id, in_port,
-   may_packet_in, honor_table_miss);
-
+   may_packet_in, honor_table_miss,
+   ctx->xin->xcache);
 if (OVS_UNLIKELY(ctx->xin->resubmit_hook)) {
 ctx->xin->resubmit_hook(ctx->xin, rule, ctx->indentation + 1);
 }
@@ -5422,7 +5422,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out 
*xout)
 ctx.rule = rule_dpif_lookup_from_table(
 ctx.xbridge->ofproto, ctx.tables_version, flow, ctx.wc,
 ctx.xin->resubmit_stats, _id,
-flow->in_port.ofp_port, true, true);
+flow->in_port.ofp_port, true, true, ctx.xin->xcache);
 if (ctx.xin->resubmit_stats) {
 rule_dpif_credit_stats(ctx.rule, ctx.xin->resubmit_stats);
 }
@@ -5752,6 +5752,14 @@ xlate_push_stats_entry(struct xc_entry *entry,
 struct eth_addr dmac;
 
 switch (entry->type) {
+case XC_TABLE:
+ofproto_dpif_credit_table_stats(entry->u.table.ofproto,
+entry->u.table.id,
+entry->u.table.match
+? stats->n_packets : 0,
+entry->u.table.match
+? 0 : stats->n_packets);
+break;
 case XC_RULE:
 rule_dpif_credit_stats(entry->u.rule, stats);
 break;
@@ -5837,6 +5845,8 @@ void
 xlate_cache_clear_entry(struct xc_entry *entry)
 {
 switch (entry->type) {
+case XC_TABLE:
+break;
 case XC_RULE:
 rule_dpif_unref(entry->u.rule);
 break;
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 776e2ad..c21cd7f 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -37,6 +37,7 @@ struct mac_learning;
 struct mcast_snooping;
 
 enum xc_type {
+XC_TABLE,
 XC_RULE,
 XC_BOND,
 XC_NETDEV,
@@ -57,6 +58,11 @@ enum xc_type {
 struct xc_entry {
 enum xc_type type;
 union {
+struct {
+struct ofproto_dpif *ofproto;
+uint8_t id;
+boolmatch; /* or miss. */
+} table;
 struct rule_dpif *rule;
 struct {
 struct netdev *tx;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index d928f01..79ffbfd 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3914,6 +3914,21 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, 
ovs_version_t version,
flow, wc)));
 }
 
+void
+ofproto_dpif_credit_table_stats(struct ofproto_dpif *ofproto, uint8_t table_id,
+uint64_t n_matches, uint64_t n_misses)
+{
+struct oftable *tbl = >up.tables[table_id];
+unsigned long orig;
+
+if (n_matches) {
+atomic_add_relaxed(>n_matched, n_matches, );
+}
+if (n_misses) {
+atomic_add_relaxed(>n_missed, n_misses, );
+}
+}
+
 /* Look up 'flow' in 'ofproto''s classifier version 'version', starting from
  * table '*table_id'.  Returns the rule that was found, which may be one of the
  * special rules according to packet miss hadling.  If 'may_packet_in' is
@@ -3945,7 +3960,8 @@ rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
 struct flow_wildcards *wc,
 const struct dpif_flow_stats *stats,
 uint8_t *table_id, ofp_port_t in_port,
-bool may_packet_in, bool honor_table_miss)
+bool may_packet_in, bool honor_table_miss,
+struct xlate_cache *xcache)
 {
 ovs_be16 old_tp_src = flow->tp_src, old_tp_dst = flow->tp_dst;
 ofp_port_t old_in_port = flow->in_port.ofp_port;
@@ -3971,6 

[ovs-dev] [PATCH v2 13/15] coverage: Rename init functions to avoid symbol collisions.

[Jarno Rajahalme]

ofproto now uses various *_init() functions, so use something else for
coverage constructors.

Signed-off-by: Jarno Rajahalme 
---
 lib/coverage.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/coverage.h b/lib/coverage.h
index 34a04aa..6d49fe0 100644
--- a/lib/coverage.h
+++ b/lib/coverage.h
@@ -75,7 +75,7 @@ void coverage_counter_register(struct coverage_counter*);
 extern struct coverage_counter counter_##COUNTER;   \
 struct coverage_counter counter_##COUNTER   \
 = { #COUNTER, COUNTER##_count, 0, 0, {0}, {0} };\
-OVS_CONSTRUCTOR(COUNTER##_init) {   \
+OVS_CONSTRUCTOR(COUNTER##_init__) { \
 coverage_counter_register(_##COUNTER);  \
 }
 
-- 
2.1.4

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

[ovs-dev] [PATCH v2 11/15] ofproto: Use ofproto_flow_mod for learn execution from xlate cache.

[Jarno Rajahalme]

Use ofproto_flow_mod with a reference to an existing or new rule
instead of ofputil_flow_mod for learn action execution from xlate
cache

Typically we would find that when a learn xlate cache entry is
created, a preceding upcall has already created the learned flow.  In
this case the xlate cache entry takes a reference to that flow and
keeps refreshing it without needing to perform any flow table lookups.
This is both faster and shrinks the memory cost of each learn cache
entry from ~3.5kb to about 0.3kb.

If the learned rule does not yet exist, it is created and attached to
the ofproto_flow_mod, from which it is then added.  If the referred
rule happens to expire and is removed from the classifier tables, we
create a new rule using the old rule as a template, so that we can
avoid storing the ofputil_flow_mod in all cases.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif-xlate.c |  61 ++-
 ofproto/ofproto-dpif-xlate.h |   4 +-
 ofproto/ofproto-dpif.c   |  17 ++--
 ofproto/ofproto-dpif.h   |   5 +-
 ofproto/ofproto-provider.h   |   6 ++
 ofproto/ofproto.c| 233 ---
 6 files changed, 233 insertions(+), 93 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 6be78e6..2683464 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4068,37 +4068,42 @@ xlate_bundle_action(struct xlate_ctx *ctx,
 }
 
 static void
-xlate_learn_action__(struct xlate_ctx *ctx, const struct ofpact_learn *learn,
- struct ofputil_flow_mod *fm, struct ofpbuf *ofpacts)
-{
-learn_execute(learn, >xin->flow, fm, ofpacts);
-if (ctx->xin->may_learn) {
-ofproto_dpif_flow_mod(ctx->xbridge->ofproto, fm);
-}
-}
-
-static void
 xlate_learn_action(struct xlate_ctx *ctx, const struct ofpact_learn *learn)
 {
 learn_mask(learn, ctx->wc);
 
-if (ctx->xin->xcache) {
-struct xc_entry *entry;
-
-entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
-entry->u.learn.ofproto = ctx->xbridge->ofproto;
-entry->u.learn.fm = xmalloc(sizeof *entry->u.learn.fm);
-entry->u.learn.ofpacts = ofpbuf_new(64);
-xlate_learn_action__(ctx, learn, entry->u.learn.fm,
- entry->u.learn.ofpacts);
-} else if (ctx->xin->may_learn) {
+if (ctx->xin->xcache || ctx->xin->may_learn) {
 uint64_t ofpacts_stub[1024 / 8];
 struct ofputil_flow_mod fm;
+struct ofproto_flow_mod ofm__, *ofm;
 struct ofpbuf ofpacts;
+enum ofperr error;
+
+if (ctx->xin->xcache) {
+struct xc_entry *entry;
+
+entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
+entry->u.learn.ofm = xmalloc(sizeof *entry->u.learn.ofm);
+ofm = entry->u.learn.ofm;
+} else {
+ofm = __;
+}
 
 ofpbuf_use_stub(, ofpacts_stub, sizeof ofpacts_stub);
-xlate_learn_action__(ctx, learn, , );
+learn_execute(learn, >xin->flow, , );
+error = ofproto_dpif_flow_mod_init_for_learn(ctx->xbridge->ofproto,
+ , ofm);
 ofpbuf_uninit();
+
+if (!error && ctx->xin->may_learn) {
+error = ofproto_flow_mod_learn(ofm, ctx->xin->xcache != NULL);
+}
+
+if (error) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(, "%s: LEARN action execution failed.",
+ ctx->xbridge->name);
+}
 }
 }
 
@@ -5779,9 +5784,15 @@ xlate_push_stats_entry(struct xc_entry *entry,
 entry->u.mirror.mirrors,
 stats->n_packets, stats->n_bytes);
 break;
-case XC_LEARN:
-ofproto_dpif_flow_mod(entry->u.learn.ofproto, entry->u.learn.fm);
+case XC_LEARN: {
+enum ofperr error;
+error = ofproto_flow_mod_learn(entry->u.learn.ofm, true);
+if (error) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(, "xcache LEARN action execution failed.");
+}
 break;
+}
 case XC_NORMAL:
 xlate_cache_normal(entry->u.normal.ofproto, entry->u.normal.flow,
entry->u.normal.vlan);
@@ -5864,8 +5875,8 @@ xlate_cache_clear_entry(struct xc_entry *entry)
 mbridge_unref(entry->u.mirror.mbridge);
 break;
 case XC_LEARN:
-free(entry->u.learn.fm);
-ofpbuf_delete(entry->u.learn.ofpacts);
+ofproto_flow_mod_uninit(entry->u.learn.ofm);
+free(entry->u.learn.ofm);
 break;
 case XC_NORMAL:
 free(entry->u.normal.flow);
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index c21cd7f..7faa199 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -84,9 +84,7 @@ struct xc_entry {
 

[ovs-dev] [PATCH v2 07/15] object-collection: Remove access to stub.

[Jarno Rajahalme]

Better not use access to the *_collection_stub(), as it is an internal
implementation detail.

Signed-off-by: Jarno Rajahalme 
---
 lib/object-collection.h |  5 -
 ofproto/ofproto.c   | 10 +-
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/lib/object-collection.h b/lib/object-collection.h
index a0ad0f2..4903451 100644
--- a/lib/object-collection.h
+++ b/lib/object-collection.h
@@ -78,11 +78,6 @@ static inline TYPE* NAME##_collection_##NAME##s(const struct 
NAME##_collection *
 return (TYPE*)coll->collection.objs;\
 }   \
 \
-static inline TYPE* NAME##_collection_stub(struct NAME##_collection *coll) \
-{   \
-return (TYPE*)coll->collection.stub;\
-}   \
-\
 static inline size_t NAME##_collection_n(const struct NAME##_collection *coll) 
\
 {   \
 return coll->collection.n;  \
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 97ed61a..2971814 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -4692,7 +4692,7 @@ add_flow_start(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
 rule_collection_add(>old_rules, old_rule);
 }
 /* Take ownership of the temp_rule. */
-rule_collection_stub(>new_rules)[0] = new_rule;
+rule_collection_add(>new_rules, new_rule);
 ofm->temp_rule = NULL;
 
 replace_rule_start(ofproto, ofm, old_rule, new_rule);
@@ -4705,8 +4705,8 @@ add_flow_revert(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm)
 OVS_REQUIRES(ofproto_mutex)
 {
 struct rule *old_rule = rule_collection_n(>old_rules)
-? rule_collection_stub(>old_rules)[0] : NULL;
-struct rule *new_rule = rule_collection_stub(>new_rules)[0];
+? rule_collection_rules(>old_rules)[0] : NULL;
+struct rule *new_rule = rule_collection_rules(>new_rules)[0];
 
 replace_rule_revert(ofproto, old_rule, new_rule);
 }
@@ -4718,8 +4718,8 @@ add_flow_finish(struct ofproto *ofproto, struct 
ofproto_flow_mod *ofm,
 OVS_REQUIRES(ofproto_mutex)
 {
 struct rule *old_rule = rule_collection_n(>old_rules)
-? rule_collection_stub(>old_rules)[0] : NULL;
-struct rule *new_rule = rule_collection_stub(>new_rules)[0];
+? rule_collection_rules(>old_rules)[0] : NULL;
+struct rule *new_rule = rule_collection_rules(>new_rules)[0];
 struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(_cookies);
 
 replace_rule_finish(ofproto, ofm, req, old_rule, new_rule, _cookies);
-- 
2.1.4

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

[ovs-dev] [PATCH v2 05/15] ofproto: Change rule's 'removed' member to a tri-state 'state'.

[Jarno Rajahalme]

As a rule may not be re-inserted to ofproto data structures, it is
cleaner to have three states for the rule, rather than just two.  This
will be useful for managing learned flows in later patches.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-provider.h | 11 +--
 ofproto/ofproto.c  | 18 +++---
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7f7813e..0013c5d 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -323,6 +323,13 @@ struct oftable {
  *  'rule->mutex', and safely written only by coding holding ofproto_mutex
  *  AND 'rule->mutex'.  These are marked OVS_GUARDED.
  */
+enum OVS_PACKED_ENUM rule_state {
+RULE_INITIALIZED, /* Rule has been initialized, but not inserted to the
+   * ofproto data structures. */
+RULE_INSERTED,/* Rule has been inserted to ofproto data structures. */
+RULE_REMOVED, /* Rule has been removed from ofproto data structures. */
+};
+
 struct rule {
 /* Where this rule resides in an OpenFlow switch.
  *
@@ -330,8 +337,6 @@ struct rule {
 struct ofproto *const ofproto; /* The ofproto that contains this rule. */
 const struct cls_rule cr;  /* In owning ofproto's classifier. */
 const uint8_t table_id;/* Index in ofproto's 'tables' array. */
-bool removed;  /* Rule has been removed from the ofproto
-* data structures. */
 
 /* Protects members marked OVS_GUARDED.
  * Readers only need to hold this mutex.
@@ -346,6 +351,8 @@ struct rule {
  * reference. */
 struct ovs_refcount ref_count;
 
+enum rule_state state OVS_GUARDED;
+
 /* A "flow cookie" is the OpenFlow name for a 64-bit value associated with
  * a flow.. */
 ovs_be64 flow_cookie OVS_GUARDED;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 8d90f7e..48d7b8f 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1474,7 +1474,7 @@ ofproto_rule_delete(struct ofproto *ofproto, struct rule 
*rule)
  * be killed. */
 ovs_mutex_lock(_mutex);
 
-if (!rule->removed) {
+if (rule->state == RULE_INSERTED) {
 /* Make sure there is no postponed removal of the rule. */
 ovs_assert(cls_rule_visible_in_version(>cr, OVS_VERSION_MAX));
 
@@ -4796,7 +4796,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct 
cls_rule *cr,
 return error;
 }
 
-rule->removed = true;   /* Not yet in ofproto data structures. */
+rule->state = RULE_INITIALIZED;
 
 *new_rule = rule;
 return 0;
@@ -8074,7 +8074,8 @@ ofproto_rule_insert__(struct ofproto *ofproto, struct 
rule *rule)
 {
 const struct rule_actions *actions = rule_get_actions(rule);
 
-ovs_assert(rule->removed);
+/* A rule may not be reinserted. */
+ovs_assert(rule->state == RULE_INITIALIZED);
 
 if (rule->hard_timeout || rule->idle_timeout) {
 ovs_list_insert(>expirable, >expirable);
@@ -8096,8 +8097,9 @@ ofproto_rule_insert__(struct ofproto *ofproto, struct 
rule *rule)
 group_add_rule(group, rule);
 }
 }
-
-rule->removed = false;
+ovs_mutex_lock(>mutex);
+rule->state = RULE_INSERTED;
+ovs_mutex_unlock(>mutex);
 }
 
 /* Removes 'rule' from the ofproto data structures.  Caller may have deferred
@@ -8106,7 +8108,7 @@ static void
 ofproto_rule_remove__(struct ofproto *ofproto, struct rule *rule)
 OVS_REQUIRES(ofproto_mutex)
 {
-ovs_assert(!rule->removed);
+ovs_assert(rule->state == RULE_INSERTED);
 
 cookies_remove(ofproto, rule);
 
@@ -8142,7 +8144,9 @@ ofproto_rule_remove__(struct ofproto *ofproto, struct 
rule *rule)
 }
 }
 
-rule->removed = true;
+ovs_mutex_lock(>mutex);
+rule->state = RULE_REMOVED;
+ovs_mutex_unlock(>mutex);
 }
 
 /* unixctl commands. */
-- 
2.1.4

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

[ovs-dev] [PATCH v2 06/15] ofproto: Remove stale comment.

[Jarno Rajahalme]

The previous line tells that this comment is now stale. Remove it.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 48d7b8f..97ed61a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -4877,8 +4877,6 @@ replace_rule_revert(struct ofproto *ofproto,
 OVS_NOT_REACHED();
 }
 ofproto_rule_remove__(ofproto, new_rule);
-/* The rule was not inserted to the ofproto provider, so we can
- * release it without deleting it from the ofproto provider. */
 ofproto_rule_unref(new_rule);
 }
 
-- 
2.1.4

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

[ovs-dev] [PATCH v2 04/15] ofproto-dpif: Remove prototype for an undefined function.

[Jarno Rajahalme]

We do not have a 'ofproto_dpif_refresh_rule()' function.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-dpif.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index f1e1209..a3b1d6b 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -161,7 +161,6 @@ int ofproto_dpif_send_packet(const struct ofport_dpif *, 
bool oam,
  struct dp_packet *);
 void ofproto_dpif_flow_mod(struct ofproto_dpif *,
const struct ofputil_flow_mod *);
-struct rule_dpif *ofproto_dpif_refresh_rule(struct rule_dpif *);
 
 struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t);
 struct ofport_dpif *ofp_port_to_ofport(const struct ofproto_dpif *,
-- 
2.1.4

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

[ovs-dev] [PATCH v2 02/15] lib: Retire packet buffering feature.

[Jarno Rajahalme]

OVS implementation of buffering packets that are sent to the
controller is not compliant with the OpenFlow specifications after
OpenFlow 1.0, which is possibly true since OpenFlow 1.0 is not really
specifying the packet buffering behavior.

OVS implementation executes the buffered packet against the actions of
the modified or added rule, whereas OpenFlow (since 1.1) specifies
that the packet should be matched against the flow table 0 and
processed accordingly.

Rather than fix this behavior, and potentially break OVS users, we
propose to remove the feature altogether. After all, such packet
buffering is an optional OpenFlow feature, and as such any possible
users should continue to work without this feature.

This patch also makes OVS check the received 'buffer_id' values more
rigorously, and fixes some internal users accordingly.

Signed-off-by: Jarno Rajahalme 
---
 FAQ.md |  49 -
 include/openvswitch/ofp-util.h |   4 +-
 lib/automake.mk|   2 -
 lib/learn.c|   2 +-
 lib/ofp-util.c |  31 +-
 lib/pktbuf.c   | 220 -
 lib/pktbuf.h   |  40 
 ofproto/bundles.c  |   1 -
 ofproto/connmgr.c  |  21 +---
 ofproto/connmgr.h  |   3 -
 ofproto/fail-open.c|   1 -
 ofproto/ofproto-dpif.c |  21 +---
 ofproto/ofproto-provider.h |  28 --
 ofproto/ofproto.c  | 142 +++---
 ovn/controller/ofctrl.c|   4 +
 tests/ofproto.at   |  10 +-
 tutorial/OVN-Tutorial.md   |   4 +-
 vswitchd/ovs-vswitchd.8.in |   5 +-
 18 files changed, 36 insertions(+), 552 deletions(-)
 delete mode 100644 lib/pktbuf.c
 delete mode 100644 lib/pktbuf.h

diff --git a/FAQ.md b/FAQ.md
index 43d5ba5..2a913f8 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -1970,55 +1970,6 @@ A: Reconfiguring your bridge can change your bridge's 
datapath-id because
 
   ovs-vsctl set bridge br0 other-config:datapath-id=0123456789abcdef
 
-### Q: My controller is getting errors about "buffers".  What's going on?
-
-A: When a switch sends a packet to an OpenFlow controller using a
-   "packet-in" message, it can also keep a copy of that packet in a
-   "buffer", identified by a 32-bit integer "buffer_id".  There are
-   two advantages to buffering.  First, when the controller wants to
-   tell the switch to do something with the buffered packet (with a
-   "packet-out" OpenFlow request), it does not need to send another
-   copy of the packet back across the OpenFlow connection, which
-   reduces the bandwidth cost of the connection and improves latency.
-   This enables the second advantage: the switch can optionally send
-   only the first part of the packet to the controller (assuming that
-   the switch only needs to look at the first few bytes of the
-   packet), further reducing bandwidth and improving latency.
-
-   However, buffering introduces some issues of its own.  First, any
-   switch has limited resources, so if the controller does not use a
-   buffered packet, the switch has to decide how long to keep it
-   buffered.  When many packets are sent to a controller and buffered,
-   Open vSwitch can discard buffered packets that the controller has
-   not used after as little as 5 seconds.  This means that
-   controllers, if they make use of packet buffering, should use the
-   buffered packets promptly.  (This includes sending a "packet-out"
-   with no actions if the controller does not want to do anything with
-   a buffered packet, to clear the packet buffer and effectively
-   "drop" its packet.)
-
-   Second, packet buffers are one-time-use, meaning that a controller
-   cannot use a single packet buffer in two or more "packet-out"
-   commands.  Open vSwitch will respond with an error to the second
-   and subsequent "packet-out"s in such a case.
-
-   Finally, a common error early in controller development is to try
-   to use buffer_id 0 in a "packet-out" message as if 0 represented
-   "no buffered packet".  This is incorrect usage: the buffer_id with
-   this meaning is actually 0x.
-
-   ovs-vswitchd(8) describes some details of Open vSwitch packet
-   buffering that the OpenFlow specification requires implementations
-   to document.
-
-   Note that the packet buffering support is slated for deprecation in
-   OVS 2.7 release.  After the change OVS always sends the 'buffer_id'
-   as 0x in "packet-in" messages and will send an error
-   response if any other value of this field is included in
-   "packet-out" and "flow mod" sent by a controller.  Controllers are
-   already expected to work properly in cases where the switch can not
-   buffer packets, so this change should not affect existing users.
-
 ### Q: How does OVS divide flows among buckets in an OpenFlow "select" group?
 
 A: In Open vSwitch 2.3 and earlier, Open vSwitch used the 

[ovs-dev] [PATCH v2 03/15] types.h: Move mirror_mask_t here.

[Jarno Rajahalme]

Move mirror_mask_t from ofproto/ofproto-dpif-mirror.h to
openvswitch/types.h to avoid including function definitions with
colliding names (e.g., mirror_destroy(), which is also the name of a
static function in bridge.c).

Signed-off-by: Jarno Rajahalme 
---
 include/openvswitch/types.h   | 5 +
 ofproto/ofproto-dpif-mirror.h | 4 +---
 ofproto/ofproto-dpif-rid.h| 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/openvswitch/types.h b/include/openvswitch/types.h
index 2f5fcca..4c6431e 100644
--- a/include/openvswitch/types.h
+++ b/include/openvswitch/types.h
@@ -152,4 +152,9 @@ struct eth_addr {
 };
 };
 
+/* Moved from ofproto/ofproto-dpif-mirror.h to avoid including the whole file,
+ * which introduces function name collisions. */
+#define MAX_MIRRORS 32
+typedef uint32_t mirror_mask_t;
+
 #endif /* openvswitch/types.h */
diff --git a/ofproto/ofproto-dpif-mirror.h b/ofproto/ofproto-dpif-mirror.h
index b00536a..5c2ed82 100644
--- a/ofproto/ofproto-dpif-mirror.h
+++ b/ofproto/ofproto-dpif-mirror.h
@@ -18,9 +18,7 @@
 #include 
 
 #include "util.h"
-
-#define MAX_MIRRORS 32
-typedef uint32_t mirror_mask_t;
+#include "openvswitch/types.h"
 
 struct ofproto_dpif;
 struct ofbundle;
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 3bca817..d086871 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -21,9 +21,9 @@
 #include 
 
 #include "cmap.h"
-#include "ofproto-dpif-mirror.h"
 #include "openvswitch/list.h"
 #include "openvswitch/ofp-actions.h"
+#include "openvswitch/types.h"
 #include "ovs-thread.h"
 #include "uuid.h"
 
-- 
2.1.4

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

[ovs-dev] [PATCH v2 01/15] vswitchd: Deprecate packet buffering in OVS 2.7.

[Jarno Rajahalme]

OVS implementation of buffering packets that are sent to the
controller is not compliant with the OpenFlow specifications after
OpenFlow 1.0.  OVS implementation executes the buffered packet against
the actions of the modified or added rule, whereas OpenFlow (since
1.1) specifies that the packet should be matched against the flow
table 0 and processed accordingly.

Rather than fix this behavior, and potentially break OVS users, we
propose to remove the feature altogether, starting in OVS 2.7.  This
patch announces this in 'NEWS', and adds detial to the FAQ question
about backet buffering.

Signed-off-by: Jarno Rajahalme 
---
 FAQ.md | 8 
 NEWS   | 8 
 2 files changed, 16 insertions(+)

diff --git a/FAQ.md b/FAQ.md
index 6d23443..43d5ba5 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -2011,6 +2011,14 @@ A: When a switch sends a packet to an OpenFlow 
controller using a
buffering that the OpenFlow specification requires implementations
to document.
 
+   Note that the packet buffering support is slated for deprecation in
+   OVS 2.7 release.  After the change OVS always sends the 'buffer_id'
+   as 0x in "packet-in" messages and will send an error
+   response if any other value of this field is included in
+   "packet-out" and "flow mod" sent by a controller.  Controllers are
+   already expected to work properly in cases where the switch can not
+   buffer packets, so this change should not affect existing users.
+
 ### Q: How does OVS divide flows among buckets in an OpenFlow "select" group?
 
 A: In Open vSwitch 2.3 and earlier, Open vSwitch used the destination
diff --git a/NEWS b/NEWS
index 280fc15..4c4785d 100644
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,14 @@ v2.6.0 - xx xxx 
packet to size M bytes when outputting to port N.
  * New command OFPGC_ADD_OR_MOD for OFPT_GROUP_MOD message that adds a
new group or modifies an existing groups
+ * Next OVS release (2.7) will deprecate the optional OpenFlow
+   packet buffering feature.  After the change OVS always sends
+   the 'buffer_id' as 0x in packet-in messages and will
+   send an error response if any other value of this field is
+   included in packet-out and flow mod sent by a controller.
+   Controllers are already expected to work properly in cases
+   where the switch can not buffer packets, so this change should
+   not affect existing users.
- Improved OpenFlow version compatibility for actions:
  * New OpenFlow extension to support the "group" action in OpenFlow 1.0.
  * OpenFlow 1.0 "enqueue" action now properly translated to OpenFlow 1.1+.
-- 
2.1.4

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

Re: [ovs-dev] [PATCH v05 62/72] include/uapi/linux/openvswitch.h: use __u32 from linux/types.h

[David Miller]

From: Mikko Rapeli 
Date: Mon, 22 Aug 2016 20:33:19 +0200

> Kernel uapi header are supposed to use them. Fixes userspace compile error:
> 
> linux/openvswitch.h:583:2: error: unknown type name ‘uint32_t’
> 
> Signed-off-by: Mikko Rapeli 

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

[ovs-dev] [PATCH v2 00/15] ofproto: Deprecate packet buffering and support packet-outs in bundles.

[Jarno Rajahalme]

The first patch announces the packet buffering deprecation in 2.7.
This is the only patch in the series that is targeted to 2.6.

The remaining patches are mostly small source code fixes and some
refactoring, but the patch 10/15 actually fixes a controller visible
stats regression introduced by the xlate cache.  Patch 11/15 reduces
the overhead of each translated and cached learn action from about
3.5kb to about 0.3kb.

Remainder of the series implements bundle support for PACKET_OUT
messages.

Jarno Rajahalme (15):
  vswitchd: Deprecate packet buffering in OVS 2.7.
  lib: Retire packet buffering feature.
  types.h: Move mirror_mask_t here.
  ofproto-dpif: Remove prototype for an undefined function.
  ofproto: Change rule's 'removed' member to a tri-state 'state'.
  ofproto: Remove stale comment.
  object-collection: Remove access to stub.
  connmgr: Make connmgr_wants_packet_in_on_miss() lock-free.
  ofproto-dpif-xlate: Expose xlate cache.
  ofproto-dpif-xlate: Add xlate cache type XC_TABLE.
  ofproto: Use ofproto_flow_mod for learn execution from xlate cache.
  ofproto-dpif-xlate: Allow translating without side-effects.
  coverage: Rename init functions to avoid symbol collisions.
  ofproto: Refactor packet_out handling.
  ofproto: Support packet_outs in bundles.

 FAQ.md  |  41 ---
 NEWS|  11 +-
 include/openvswitch/ofp-parse.h |   5 +
 include/openvswitch/ofp-util.h  |   5 +-
 include/openvswitch/types.h |   5 +
 lib/automake.mk |   2 -
 lib/coverage.h  |   2 +-
 lib/learn.c |   2 +-
 lib/object-collection.h |   5 -
 lib/ofp-parse.c | 108 ++
 lib/ofp-util.c  |  38 +--
 lib/pktbuf.c| 220 -
 lib/pktbuf.h|  40 ---
 ofproto/bundles.c   |   1 -
 ofproto/bundles.h   |   7 +-
 ofproto/connmgr.c   |  80 ++---
 ofproto/connmgr.h   |   3 -
 ofproto/fail-open.c |   1 -
 ofproto/ofproto-dpif-mirror.h   |   4 +-
 ofproto/ofproto-dpif-rid.h  |   2 +-
 ofproto/ofproto-dpif-upcall.c   |  11 +-
 ofproto/ofproto-dpif-xlate.c| 436 
 ofproto/ofproto-dpif-xlate.h|  95 +-
 ofproto/ofproto-dpif.c  | 202 +---
 ofproto/ofproto-dpif.h  |  30 +-
 ofproto/ofproto-provider.h  | 121 ---
 ofproto/ofproto.c   | 712 +---
 ovn/controller/ofctrl.c |   4 +
 tests/ofproto.at| 144 +++-
 tutorial/OVN-Tutorial.md|   4 +-
 utilities/ovs-ofctl.8.in|  46 ++-
 utilities/ovs-ofctl.c   |  55 
 vswitchd/ovs-vswitchd.8.in  |   5 +-
 33 files changed, 1418 insertions(+), 1029 deletions(-)
 delete mode 100644 lib/pktbuf.c
 delete mode 100644 lib/pktbuf.h

-- 
2.1.4

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

Re: [ovs-dev] [PATCH v05 38/72] include/uapi/linux/openvswitch.h: use __u32 from linux/types.h

[David Miller]

From: Mikko Rapeli 
Date: Mon, 22 Aug 2016 20:32:55 +0200

> Fixes userspace compiler error:
> 
> error: unknown type name ‘uint32_t’
> 
> Signed-off-by: Mikko Rapeli 

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

Re: [ovs-dev] [PATCH v05 38/72] include/uapi/linux/openvswitch.h: use __u32 from linux/types.h

[David Miller]

From: Stephen Hemminger 
Date: Mon, 22 Aug 2016 12:39:38 -0700

> This is a a real issue, but being buried in a huge patch series of include
> file stuff I don't think anyone would see it.

It hit netdev and thus patchwork, therefore anyone watching networking
patches saw it.

I would not call it buried at all.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] ovn: Replace tabs with spaces and clean up alignment in unit tests.

[Flavio Fernandes]

Signed-off-by: Flavio Fernandes 
---
 tests/ovn.at | 154 +--
 1 file changed, 77 insertions(+), 77 deletions(-)

diff --git a/tests/ovn.at b/tests/ovn.at
index 216bb07..e4f6c30 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -18,9 +18,9 @@ m4_divert_text([PREPARE_TESTS],
  exp_text=$2
  exp_n=`wc -l < "$exp_text"`
  ovs_wait_cond () {
- $PYTHON "$top_srcdir/utilities/ovs-pcap.in" $rcv_pcap > $rcv_text
- rcv_n=`wc -l < "$rcv_text"`
- test $rcv_n -ge $exp_n
+ $PYTHON "$top_srcdir/utilities/ovs-pcap.in" $rcv_pcap > $rcv_text
+ rcv_n=`wc -l < "$rcv_text"`
+ test $rcv_n -ge $exp_n
  }
  ovs_wait || echo "expected $exp_n packets, only received $rcv_n"
 
@@ -1243,15 +1243,15 @@ for i in 1 2 3; do
 ovn-nbctl lsp-add lsw0 lp$i
 ovn-sbctl lsp-bind lp$i hv0
 if test $i = 1; then
-   ovn-nbctl lsp-set-addresses lp$i "f0:00:00:00:00:0$i 192.168.0.$i" 
unknown
+ovn-nbctl lsp-set-addresses lp$i "f0:00:00:00:00:0$i 192.168.0.$i" unknown
 else
-   if test $i = 3; then
-   ip_addrs="192.168.0.$i fe80::ea2a:eaff:fe28:$i/64 192.169.0.$i"
-   else
-   ip_addrs="192.168.0.$i"
-   fi
-   ovn-nbctl lsp-set-addresses lp$i "f0:00:00:00:00:$i $ip_addrs"
-   ovn-nbctl lsp-set-port-security lp$i f0:00:00:00:00:$i
+if test $i = 3; then
+ip_addrs="192.168.0.$i fe80::ea2a:eaff:fe28:$i/64 192.169.0.$i"
+else
+ip_addrs="192.168.0.$i"
+fi
+ovn-nbctl lsp-set-addresses lp$i "f0:00:00:00:00:$i $ip_addrs"
+ovn-nbctl lsp-set-port-security lp$i f0:00:00:00:00:$i
 fi
 done
 ovn-nbctl acl-add lsw0 from-lport 1000 'eth.type == 0x1234' drop
@@ -1275,14 +1275,14 @@ test_packet() {
 local inport=$1 eth_dst=$2 eth_src=$3; shift; shift; shift
 uflow="inport==\"lp$inport\" && eth.dst==$eth_dst && eth.src==$eth_src"
 while :; do
-   case $1 in # (
-   -vlan) uflow="$uflow && vlan.vid == 1234"; shift ;; # (
-   -eth) uflow="$uflow && eth.type == 0x$2"; shift; shift ;; # (
-   *) break ;;
-   esac
+case $1 in # (
+-vlan) uflow="$uflow && vlan.vid == 1234"; shift ;; # (
+-eth) uflow="$uflow && eth.type == 0x$2"; shift; shift ;; # (
+*) break ;;
+esac
 done
 for outport; do
-   echo "output(\"lp$outport\");"
+echo "output(\"lp$outport\");"
 done > expout
 
 AT_CAPTURE_FILE([trace])
@@ -1305,17 +1305,17 @@ test_arp() {
 local request="inport == \"lp$inport\"
&& eth.dst == ff:ff:ff:ff:ff:ff && eth.src == $sha
&& arp.op == 1 && arp.sha == $sha && arp.spa == $spa
-  && arp.tha == ff:ff:ff:ff:ff:ff && arp.tpa == $tpa"
+   && arp.tha == ff:ff:ff:ff:ff:ff && arp.tpa == $tpa"
 
 if test -z "$reply_ha"; then
 reply=
-   local i
-   for i in 1 2 3; do
-   if test $i != $inport; then
-   reply="${reply}output(\"lp$i\");
+local i
+for i in 1 2 3; do
+if test $i != $inport; then
+reply="${reply}output(\"lp$i\");
 "
-   fi
-   done
+fi
+done
 else
 reply="\
 eth.dst = $sha;
@@ -1368,64 +1368,64 @@ for s in 1 2 3; do
 bacl2=
 bacl3=
 for d in 1 2 3; do
-   echo
-   echo "lp$s -> lp$d"
-   if test $d != $s; then unicast=$d; else unicast=; fi
-   test_packet $s f0:00:00:00:00:0$d f0:00:00:00:00:0$s $unicast  #1
-
-   if test $d != $s && test $s = 1; then
-   impersonate=$d
-   else
-   impersonate=
-   fi
-   test_packet $s f0:00:00:00:00:0$d f0:00:00:00:00:55 $impersonate   #3
-
-   if test $d != $s && test $s != 1; then acl2=$d; else acl2=; fi
-   if test $d != $s && test $d != 3; then acl3=$d; else acl3=; fi
-   if test $d = $s || ( (test $s = 1 || test $s = 2) && test $d = 3); then
-   # Source of 1 or 2 and dest of 3 should be dropped
-   # due to the 4th ACL that uses address_set(set1).
-   acl4=
-   else
-   acl4=$d
-   fi
-
-   #7, acl1 to acl4:
-   test_packet $s f0:00:00:00:00:0$d f0:00:00:00:00:0$s -eth 1234
-   test_packet $s f0:00:00:00:00:0$d f0:00:00:00:00:0$s -eth 1235 $acl2
-   test_packet $s f0:00:00:00:00:0$d f0:00:00:00:00:0$s -eth 1236 $acl3
-   test_packet $s f0:00:00:00:00:0$d f0:00:00:00:00:0$s -eth 1237 $acl4
-
-   test_packet $s f0:00:00:00:00:0$d f0:00:00:00:00:55 -vlan  #4
-   test_packet $s f0:00:00:00:00:0$d 01:00:00:00:00:0$s   #5
-
-   if test $d != $s && test $d = 1; then
-   unknown="$unknown $d"
-   fi
-   bcast="$bcast $unicast"
-   bacl2="$bacl2 $acl2"
-   bacl3="$bacl3 $acl3"
-
-   sip=192.168.0.$s
-   tip=192.168.0.$d
-   tip_unknown=11.11.11.11
-   test_arp $s f0:00:00:00:00:0$s $sip $tip f0:00:00:00:00:0$d#9
-   test_arp $s 

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

[Darrell Ball]

On Sun, Aug 14, 2016 at 3:59 AM, Itamar Ofek 
wrote:

>
> Darrell,
>
>
> To aviod lengthy  response, i'll address the issues directly.
>
>
> The code in ovs-vtep should not be Neutron L2 border gateway specific.
> Please remove references to so-called relay and mesh ports.
>
> The relay vteps are a second level of hierachy of vteps - I have more
> detail
>
>  --agreed
>
> Do not link per tunnel tunnel keys to Neutron L2 border gateway relay vteps
> in generic code, such as ovs-vtep.
>
>
> --agreed.
>
>
> Regarding vtep.ovsschema:
>
>
> diff --git a/vtep/vtep.ovsschema b/vtep/vtep.ovsschema
> index 3a24318..f41568f 100644
> --- a/vtep/vtep.ovsschema
> +++ b/vtep/vtep.ovsschema
> @@ -1,6 +1,6 @@
>  {
>"name": "hardware_vtep",
> -  "cksum": "353943336 11434",
> +  "cksum": "3167636278 11608",
>"tables": {
>  "Global": {
>"columns": {
> @@ -209,7 +209,12 @@
>"type": "string"}},
>"mutable": false},
>  "dst_ip": {"type": "string", "mutable": false},
> -"tunnel_key": {"type": {"key": "integer", "min": 0, "max": 1}}},
> +"tunnel_key": {"type": {"key": "integer", "min": 0, "max": 1}},
> +"tunnel_level": {
> +  "type": {
> +"key": {
> +  "enum": ["set", ["level0", "level1"]],
> +  "type": "string"},"min": 0, "max": 1}}},
>"indexes": [["encapsulation_type", "dst_ip", "tunnel_key"]]},
>  "ACL_entry": {
>"columns": {
>
> I have no issues with this: just  two notes the tunne_level should default
> to "level0"
>

I mentioned the default of level0 in the vtep.xml file and its also
explicit in the
ovs-vtep code.
Note, however vtep-ctl.c should not set this as explicit default in the
VTEP DB if
not supplied by the user.





> and vtep-ctl should be changed accordingly.
>
-- I can suggest a patch for it.
>

Yes, please add this parsing for tunnel_level; see below for more comments.

>
> In order to make sure the handling in ovs-vtep satisfies the l2gw I'd
> prefers having your suggested patch
>
> and run tests with it applied.
>

I am including a more comprehensive incremental patch as a suggestion
and to speed the process along.
It includes:
1) my tunnel_level suggestion and some related documentation
2) ovs-vtep support for tunnel_level and tunnel_scope_key including
   parsing changes for "list-remote-macs".
   I added tunnel_level flood set changes which follows what you did
   for "relay" and "mesh" tunnels, but I made it generic using tunnel_level.

3) Other ovs-vtep suggested changes.
4) vtep-ctl.c changes for parsing tunnel_scope_key and general parsing
handling for modified code paths.
  I removed the "[[]]" formatting for tunnel_scope_key here and in ovs-vtep.
5) Coding standard suggestions.
6) vtep-ctl.at format changes after removing the [[]]

It does NOT include vtep-ctl.c changes to parse tunnel_level and the various
cases for optional arguments (there are 3 optional arguments now) - that
needs to be added. I am assuming you will add that parsing for different
cases.
vtep-ctl.at should be updated for tunnel_level as well.

I think you should be primary owner for the patches and I can help as
needed.

diff --git a/tests/vtep-ctl.at b/tests/vtep-ctl.at
index 4178309..239560b 100644
--- a/tests/vtep-ctl.at
+++ b/tests/vtep-ctl.at
@@ -441,8 +441,8 @@ AT_CHECK([RUN_VTEP_CTL([list-local-macs ls1])], [0],
[ucast-mac-local
   00:11:22:33:44:55 -> vxlan_over_ipv4/10.0.0.10
   00:11:22:33:44:66 -> vxlan_over_ipv4/10.0.0.11
-  00:11:22:33:55:66 -> vxlan_over_ipv4/10.0.0.12 [[100]]
-  00:11:22:33:55:77 -> vxlan_over_ipv4/10.0.0.13 [[200]]
+  00:11:22:33:55:66 -> vxlan_over_ipv4/10.0.0.12 100
+  00:11:22:33:55:77 -> vxlan_over_ipv4/10.0.0.13 200

 mcast-mac-local

@@ -475,9 +475,9 @@ AT_CHECK([RUN_VTEP_CTL(
 AT_CHECK([RUN_VTEP_CTL([list-local-macs ls1])], [0],
[ucast-mac-local
   00:11:22:33:44:55 -> vxlan_over_ipv4/10.0.0.11
-  00:11:22:33:55:66 -> vxlan_over_ipv4/10.0.0.13 [[200]]
+  00:11:22:33:55:66 -> vxlan_over_ipv4/10.0.0.13 200
   00:11:22:33:55:77 -> vxlan_over_ipv4/10.0.0.15
-  00:11:22:33:55:88 -> vxlan_over_ipv4/10.0.0.17 [[400]]
+  00:11:22:33:55:88 -> vxlan_over_ipv4/10.0.0.17 400

 mcast-mac-local

@@ -501,8 +501,8 @@ AT_CHECK([RUN_VTEP_CTL([list-local-macs ls1])], [0],
[ucast-mac-local
   00:11:22:33:44:55 -> vxlan_over_ipv4/10.0.0.10
   00:11:22:33:44:66 -> vxlan_over_ipv4/10.0.0.11
-  00:11:22:33:55:66 -> vxlan_over_ipv4/10.0.0.12 [[100]]
-  00:11:22:33:55:77 -> vxlan_over_ipv4/10.0.0.13 [[200]]
+  00:11:22:33:55:66 -> vxlan_over_ipv4/10.0.0.12 100
+  00:11:22:33:55:77 -> vxlan_over_ipv4/10.0.0.13 200

 mcast-mac-local

@@ -514,7 +514,7 @@ AT_CHECK([RUN_VTEP_CTL(
 AT_CHECK([RUN_VTEP_CTL([list-local-macs ls1])], [0],
[ucast-mac-local
   00:11:22:33:44:66 -> vxlan_over_ipv4/10.0.0.11
-  00:11:22:33:55:77 -> vxlan_over_ipv4/10.0.0.13 [[200]]
+  00:11:22:33:55:77 -> vxlan_over_ipv4/10.0.0.13 200

 mcast-mac-local

@@ 

Re: [ovs-dev] [PATCH net-next v11 5/6] openvswitch: add layer 3 flow/port support

[Joe Stringer]

On 22 August 2016 at 04:04, Simon Horman  wrote:
> On Wed, Aug 10, 2016 at 10:17:30AM -0700, Joe Stringer wrote:
>> On 10 August 2016 at 03:20, Simon Horman  wrote:
>> > On Tue, Aug 09, 2016 at 08:47:40AM -0700, pravin shelar wrote:
>> >> On Mon, Aug 8, 2016 at 8:17 AM, Simon Horman  
>> >> wrote:
>> >> > Light testing seems to indicate that it works for GSO skbs
>> >> > received over both L3 and L2 GRE tunnels by OvS with both
>> >> > IP-in-MPLS and IP (without MPLS) payloads.
>> >> >
>> >>
>> >> Thanks for testing it. Can you also add those tests to OVS kmod test 
>> >> suite?
>> >> ..
>> >
>> > Sure, I will look into doing that.
>> > Am I correct in thinking Joe Stringer is the best person to contact if
>> > I run into trouble there?
>>
>> Sure. The basics of running the tests is documented here:
>> https://github.com/openvswitch/ovs/blob/master/INSTALL.md#datapath-testing
>>
>> You should be able to get a good feel for how to add tests by perusing
>> the commits to tests/system-{traffic,kmod-macros}.at in the OVS source
>> tree.
>
> Thanks Joe,
>
> it took me a while but I think that I have something working
> against the head branch of the OVS tree. I'd value opinions
> on the direction I have taken.
>
> Subject: [PATCH] system-traffic: Exercise GSO
>
> Exercise GSO for: unencapsulated; MPLS; GRE; and MPLS in GRE.
>
> There is scope to extend this testing to other encapsulation formats
> if desired.
>
> This is motivated by a desire to test GRE and MPLS encapsulation in
> the context of L3/VPN (MPLS over non-TEB GRE work). That is not
> tested here but tests for those cases would ideally be based on those in
> this patch.

This makes sense to me. There's a few corners that could be improved,
primarily for reproducing sane results on a variety of systems, then a
couple of style comments. Please do run the tests via both "make
check-kernel" and "make check-system-userspace" before submitting,
ideally with at least two varieties of kernel: One where you would
expect the test to pass, and one where you would expect the tests to
be skipped.

* CHECK_MPLS is defined in system-kmod-macros.at, so a corresponding
version should be provided in system-userspace-macros.at. If the
criteria for running the test(s) with both userspace and kernel
datapaths is the same, then this could instead be moved into
system-common-macros.at.
* "datapath - ping over gre tunnel" adds a command to execute in
at_ns1, but that namespace doesn't exist.
* "datapath - http over gre tunnel" is missing MPLS_CHECK.
* Is there a way to clear the netstat statistics before running the
tests which rely on it? I'm getting a failure on one of my systems
(ubuntu trusty with a 4.7 kernel), but I'm not sure if the counter was
already high before I ran the test.
* "datapath - http over mpls between two ports"  (maybe others too?)
should shift all openflow rules into a single section using AT_DATA,
similar to the other tests. This makes it easier to reason about the
flow table and understand what's going on before reading through the
rest of the test.
* If there is a common set of configuration you do for local stack
within a namespace to route MPLS traffic, you could consider adding
another macro into system-common-macros.at.

I also see this error on "http over mpls over gre tunnel":
+sh: 1: cannot create /proc/sys/net/mpls/conf/ns_gre0/input: Directory
nonexistent

Maybe MPLS + GRE needs a separate check?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[pravin shelar]

On Mon, Aug 22, 2016 at 2:40 PM, Jesse Gross  wrote:
> On Mon, Aug 22, 2016 at 2:24 PM, Pravin B Shelar  wrote:
>> Signed-off-by: Pravin B Shelar 
>> ---
>>  .travis.yml|  1 +
>>  .travis/linux-build.sh | 12 
>>  FAQ.md |  2 +-
>>  NEWS   |  2 +-
>>  acinclude.m4   |  4 ++--
>>  5 files changed, 17 insertions(+), 4 deletions(-)
>
> Acked-by: Jesse Gross 


Thanks.
I pushed patch to master and branch-2.6.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Jesse Gross]

On Mon, Aug 22, 2016 at 2:24 PM, Pravin B Shelar  wrote:
> Signed-off-by: Pravin B Shelar 
> ---
>  .travis.yml|  1 +
>  .travis/linux-build.sh | 12 
>  FAQ.md |  2 +-
>  NEWS   |  2 +-
>  acinclude.m4   |  4 ++--
>  5 files changed, 17 insertions(+), 4 deletions(-)

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

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

[Pravin B Shelar]

Signed-off-by: Pravin B Shelar 
---
 .travis.yml|  1 +
 .travis/linux-build.sh | 12 
 FAQ.md |  2 +-
 NEWS   |  2 +-
 acinclude.m4   |  4 ++--
 5 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 4ae6a5b..9f967ad 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -29,6 +29,7 @@ env:
   - BUILD_ENV="-m32" OPTS="--disable-ssl"
   - KERNEL=3.17.7 DPDK=1
   - KERNEL=3.17.7 DPDK=1 OPTS="--enable-shared"
+  - KERNEL=4.7.2
   - KERNEL=4.4.15
   - KERNEL=4.1.28
   - KERNEL=3.18.37
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 1b3d43d..3bcec93 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -22,6 +22,18 @@ function install_kernel()
 cd linux-${1}
 make allmodconfig
 
+# Cannot use CONFIG_KCOV: -fsanitize-coverage=trace-pc is not supported by 
compiler
+sed -i 's/CONFIG_KCOV=y/CONFIG_KCOV=n/' .config
+
+# stack validation depends on tools/objtool, but objtool does not compile 
on travis.
+# It is giving following error.
+#  >>> GEN  arch/x86/insn/inat-tables.c
+#  >>> Semantic error at 40: Unknown imm opnd: AL
+# So for now disable stack-validation for the build.
+
+sed -i 's/CONFIG_STACK_VALIDATION=y/CONFIG_STACK_VALIDATION=n/' .config
+make oldconfig
+
 # Older kernels do not include openvswitch
 if [ -d "net/openvswitch" ]; then
 make net/openvswitch/
diff --git a/FAQ.md b/FAQ.md
index f4fd55d..6d23443 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -172,7 +172,7 @@ A: The following table lists the Linux kernel versions 
against which the
 |2.3.x | 2.6.32 to 3.14
 |2.4.x | 2.6.32 to 4.0
 |2.5.x | 2.6.32 to 4.3
-|2.6.x | 3.10 to 4.6
+|2.6.x | 3.10 to 4.7
 
Open vSwitch userspace should also work with the Linux kernel module
built into Linux 3.3 and later.
diff --git a/NEWS b/NEWS
index 12788b6..280fc15 100644
--- a/NEWS
+++ b/NEWS
@@ -100,7 +100,7 @@ v2.6.0 - xx xxx 
- Datapath Linux kernel compatibility.
  * Dropped support for kernel older than 3.10.
  * Removed VLAN splinters feature.
- * Datapath supports kernel upto 4.6.
+ * Datapath supports kernel upto 4.7.
- Tunnels:
  * Flow based tunnel match and action can be used for IPv6 address using
tun_ipv6_src, tun_ipv6_dst fields.
diff --git a/acinclude.m4 b/acinclude.m4
index 8857b81..353519d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [
 AC_MSG_RESULT([$kversion])
 
 if test "$version" -ge 4; then
-   if test "$version" = 4 && test "$patchlevel" -le 6; then
+   if test "$version" = 4 && test "$patchlevel" -le 7; then
   : # Linux 4.x
else
-  AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
newer than 4.6.x is not supported (please refer to the FAQ for advice)])
+  AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
newer than 4.7.x is not supported (please refer to the FAQ for advice)])
fi
 elif test "$version" = 3 && test "$patchlevel" -ge 10; then
: # Linux 3.x
-- 
1.8.3.1

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

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis

[Lance Richardson]

> From: "Ben Pfaff" 
> To: "Russell Bryant" 
> Cc: "Lance Richardson" , "ovs dev" 
> , "Russell Bryant" 
> Sent: Monday, August 22, 2016 1:22:43 PM
> Subject: Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis
> 
> On Mon, Aug 22, 2016 at 01:14:03PM -0400, Russell Bryant wrote:
> > On Mon, Aug 22, 2016 at 12:30 PM, Ben Pfaff  wrote:
> > 
> > > On Tue, Aug 16, 2016 at 09:30:21AM -0400, Lance Richardson wrote:
> > > > As described in ovn/TODO, these are the two main approaches that could
> > > > be
> > > > used to minimize the impact of a compromised chassis on the rest of an
> > > > OVN OVN network:
> > > >
> > > >   1) Implement a role- or identity-based access control mechanism for
> > > >  ovsdb-server and use it to limit ovn-controller write access to
> > > >  tables in the southbound database.
> > > >
> > > > or
> > > >
> > > >   2) Disallow all write access to the southbound database by
> > > ovn-controller
> > > >  (as an optional mode or unconditionally) and provide alternative
> > > >  mechanisms for updating the southbound database for entries that
> > > >  are
> > > >  currently updated by ovn-controller.
> > > >
> > > > It is believed that option (1) would require somewhat more effort than
> > > (2),
> > > > and, because it would involve significant modifications to
> > > > ovsdb-server,
> > > > would also be more likely to add risk and burden to non-OVN users.
> > > > Additionally, option (2) will likely place fewer requirements on
> > > alternative
> > > > databases (such as etcd), so the following implementation discussion
> > > > only
> > > > considers option (2).
> > >
> > > I've always pushed back against adding granular access control
> > > mechanisms to OVSDB because I didn't believe it was likely that anything
> > > that was simple enough to be in the "spirit of OVSDB" (heh) was also
> > > going to be sufficient to fit a real use case.  However, if we do now
> > > have specific requirements for OVN, then I'd invite descriptions of what
> > > access control mechanism would be sufficient.  If it's simple and
> > > general enough, then implementing it in OVSDB might totally make sense.
> > >
> > > I don't think that the "risk and burden" of a simple and general
> > > mechanism is a real issue.
> > 
> > 
> > I think that push back makes sense.
> > 
> > The proposal here was to take route #2.  The only OVSDB feature required in
> > that case is to accept read-only connections, which could be on a
> > per-socket basis.  This seems much simpler all around, as long as we can
> > all get on board with ovn-controller as a read-only client.
> 
> I'm not actually saying we should choose #1.  I'm saying a couple of
> things.  First, changing OVSDB is not a huge deal; we do it when it
> makes sense.  Second, that it is possible that our specific application
> here is a better place to start for OVSDB access control than a blanket
> "we need access control for OVSDB" that I've heard a couple of times.
> 

Based on my own narrow view of the world, I think option #1 would need:

   - The ability for ovsdb-server to associate a role/identity with each
 client connection.  For simplicity this could be a binary "privileged"
 vs "non-privileged" association, perhaps using per-role SSL certificates
 for TLS connections and treating unix socket connections as "privileged".
   - A mechanism for mapping a role/identity to access rights on a per-table
 and per-column basis.
   - A mechanism for enforcing access rights on a per-table or per-column basis,
 in some cases also considering the identity of the client that created
 the row.

This infrastructure would be applied to OVN to implement the following:
- These tables would be read-only for non-privileged clients:
  SB_Global, Logical_Flow, Multicast_Group, Datapath_Binding, Address_Set,
  DHCP_Options, and DHCPv6_Options.

- The Chassis and Encap tables would allow insertions by non-privileged 
clients
  and updates to existing rows only for the clients that inserted them.

- The Port_Binding table would be writable only by privileged clients
  (ovn-northd) except for the "Chassis" column which should be writable by 
any
  non-privileged client (note that this doesn't do a lot to minimize harm 
from
  a compromised chassis).

- The MAC_Binding table should be writable by any non-privileged client 
(which also
  doesn't do much to minimize harm from a compromised chassis).

> > Are you interested in looking closer at what #1 would look like, with
> > details of what the access control policy would look like?
> 
> It'll probably be obvious, or close to obvious, what would be needed for
> #1 once we talk through what #2 needs.
>
 
Here's a slightly more detailed breakdown of the work needed for option #2:

ovsdb-server: Add support for 

Re: [ovs-dev] [PATCH 1/3] OVN-Tutorial: Replace example with ASCII quotes.

[Flavio Fernandes]

> On Aug 22, 2016, at 2:39 PM, Lance Richardson  wrote:
> 
>> From: "Justin Pettit" 
>> To: dev@openvswitch.org
>> Sent: Thursday, August 11, 2016 2:09:12 AM
>> Subject: [ovs-dev] [PATCH 1/3] OVN-Tutorial: Replace example with ASCII  
>> quotes.
>> 
>> The "--ovn" argument for SANDBOXFLAGS used unicode quotes, which when
>> copy and pasted made the command mysteriously fail.
>> 
>> Signed-off-by: Justin Pettit 
>> ---
>> tutorial/OVN-Tutorial.md | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tutorial/OVN-Tutorial.md b/tutorial/OVN-Tutorial.md
>> index be9805c..65e4ea2 100644
>> --- a/tutorial/OVN-Tutorial.md
>> +++ b/tutorial/OVN-Tutorial.md
>> @@ -17,7 +17,7 @@ section of [Tutorial.md].
>> pass the `--ovn` flag.  For example, if running it straight from the ovs git
>> tree you would run:
>> 
>> -$ make sandbox SANDBOXFLAGS=”--ovn”
>> +$ make sandbox SANDBOXFLAGS="--ovn"
>> 
>> Running the sandbox with OVN enabled does the following additional steps to
>> the
>> environment:
> 
> Nice, I fell into this trap the first time I tried to run the ovn tutorials.

+1  :)

> 
> Acked-by: Lance Richardson >
> ___
> 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 v05 38/72] include/uapi/linux/openvswitch.h: use __u32 from linux/types.h

[Stephen Hemminger]

On Mon, 22 Aug 2016 20:32:55 +0200
Mikko Rapeli  wrote:

> Fixes userspace compiler error:
> 
> error: unknown type name ‘uint32_t’
> 
> Signed-off-by: Mikko Rapeli 
> ---
>  include/uapi/linux/openvswitch.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index d95a301..645499a 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -632,8 +632,8 @@ enum ovs_hash_alg {
>   * @hash_basis: basis used for computing hash.
>   */
>  struct ovs_action_hash {
> - uint32_t  hash_alg; /* One of ovs_hash_alg. */
> - uint32_t  hash_basis;
> + __u32  hash_alg; /* One of ovs_hash_alg. */
> + __u32  hash_basis;
>  };
>  
>  /**

This is a a real issue, but being buried in a huge patch series of include
file stuff I don't think anyone would see it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Are there plans to add options for finetuning SSL server connection options?

[Ethan Rahn]

Ben,

Thanks for the response. I just submitted a patch to add those options.

Cheers,

Ethan

On Mon, Aug 15, 2016 at 11:24 AM, Ben Pfaff  wrote:

> On Mon, Aug 15, 2016 at 11:22:32AM -0700, Ethan Rahn wrote:
> > Currently the ovs-server's set of SSL server options seem related to
> which
> > certificates to use. Under stream-ssl.c I see that there's no ability to
> > configure the TLS protocol nor ciphersuites used. I'm considering adding
> > these configuration options, but I want to make sure that I'm not going
> to
> > be duplicating anyone else's work before I begin.
> >
> > I checked and I see that this is listed as part of the TODO list, and I
> > don't see any pull requests or discussion in the past year or so about
> > this. Please let me know if this is already being worked on by someone
> else.
>
> That would be useful.  I don't know of anyone else working on that.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] Add support for specifying SSL connection parameters to ovsdb

[Ethan Rahn]

From ae9961aa2521ebd5cfeef28812d8a089b7b5e55b Mon Sep 17 00:00:00 2001
From: Ethan Rahn 
Date: Mon, 22 Aug 2016 11:26:54 -0700
Subject: [PATCH] Add support for specifying SSL connection parameters to
ovsdb

OVSDB currently does not support fine-tuning the SSL parameters used for
connections. This
means that users are unable to specify not using ciphers widely considered
to be unsafe or
to avoid using TLS protocols that do not meet their organizational
standards.

This adds two new commands “—-ssl-protocols” and “—-ssl-ciphers” to the
ovsdb programs to
specify which SSL protocols and ciphers to use. In addition, the default
cipher string is set
to "HIGH:!aNULL:!MD5”. This is the current default for nginx and removes
weak ciphers while
allowing most services from the last several years to still connect.

The patch was tested by adding new test cases that check that the options
can be set and that
when incompatible SSL parameters are used that it results in a failure to
communicate. Additionally,
since this adds 2 new files, “make distcheck” was used to verify that this
works correctly.

Signed-off-by: Ethan Rahn 
---
 AUTHORS   |  1 +
 lib/automake.mk   |  2 +
 lib/ssl-connect-syn.man   |  5 +++
 lib/ssl-connect.man   | 16 +++
 lib/stream-ssl.c  | 70
+++
 lib/stream-ssl.h  | 20 -
 manpages.mk   |  8 
 ovn/controller-vtep/ovn-controller-vtep.c |  3 +-
 ovn/controller/ovn-controller.c   |  3 +-
 ovn/northd/ovn-northd.c   |  1 +
 ovn/utilities/ovn-nbctl.c |  3 +-
 ovn/utilities/ovn-sbctl.c |  3 +-
 ovn/utilities/ovn-trace.c |  1 +
 ovsdb/ovsdb-client.1.in   |  3 ++
 ovsdb/ovsdb-client.c  |  3 +-
 ovsdb/ovsdb-server.1.in   |  3 ++
 ovsdb/ovsdb-server.c  | 23 --
 tests/ovsdb-server.at | 68
+-
 tests/test-jsonrpc.c  |  3 +-
 utilities/ovs-ofctl.c |  3 +-
 utilities/ovs-testcontroller.c|  3 +-
 utilities/ovs-vsctl.c |  3 +-
 vswitchd/ovs-vswitchd.c   |  1 +
 vtep/vtep-ctl.c   |  3 +-
 24 files changed, 234 insertions(+), 18 deletions(-)
 create mode 100644 lib/ssl-connect-syn.man
 create mode 100644 lib/ssl-connect.man

diff --git a/AUTHORS b/AUTHORS
index c089d59..197142f 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -80,6 +80,7 @@ Eitan Eliahuelia...@vmware.com
 Eohyung Lee liquidnu...@gmail.com
 Eric Sesterhenn eric.sesterh...@lsexperts.de
 Ethan J. Jacksone...@eecs.berkeley.edu
+Ethan Rahn  er...@arista.com
 Eziz Durdyyev   ezizdu...@gmail.com
 Flavio Fernandesfla...@flaviof.com
 Flavio Leitner  f...@redhat.com
diff --git a/lib/automake.mk b/lib/automake.mk
index 165e6a8..62bb17b 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -462,6 +462,8 @@ MAN_FRAGMENTS += \
  lib/ssl-peer-ca-cert-syn.man \
  lib/ssl.man \
  lib/ssl-syn.man \
+ lib/ssl-connect.man \
+ lib/ssl-connect-syn.man \
  lib/table.man \
  lib/unixctl.man \
  lib/unixctl-syn.man \
diff --git a/lib/ssl-connect-syn.man b/lib/ssl-connect-syn.man
new file mode 100644
index 000..0510a59
--- /dev/null
+++ b/lib/ssl-connect-syn.man
@@ -0,0 +1,5 @@
+.IP "SSL connection options:"
+[\fB\-\-ssl\-protocols=\fITLSv1,TLSv1.1,TLSv1.2\fR]
+.br
+[\fB\-\-ssl\-ciphers=\fIHIGH:!aNULL:!MD5\fR]
+.br
diff --git a/lib/ssl-connect.man b/lib/ssl-connect.man
new file mode 100644
index 000..dcc6a79
--- /dev/null
+++ b/lib/ssl-connect.man
@@ -0,0 +1,16 @@
+.de IQ
+.  br
+.  ns
+.  IP "\\$1"
+..
+.IQ "\fB\-\-ssl\-protocols=\fITLSv1,TLSv1.1,TLSv1.2\fR"
+Specifies, in a comma or white-list delimited, list the SSL protocols
\fB\*(PN\fR
+will support for SSL connections. Supported protocols are: TLSv1, TLSv1.1,
+TLSv1.2. Order does not matter, the highest protocol supported by both
sides
+will be choosen when making the connection.
+.
+.IQ "\fB\-\-ssl\-ciphers=\fIHIGH:!aNULL:!MD5\fR"
+Specifies, in OpenSSL cipher string format, the ciphers \fB\*(PN\fR will
+support for SSL connections.
+
+
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index a5c32a1..87b8de9 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -162,6 +162,8 @@ struct ssl_config_file {
 static struct ssl_config_file private_key;
 static struct ssl_config_file certificate;
 static struct ssl_config_file ca_cert;
+static char *ssl_protocols = "TLSv1,TLSv1.1,TLSv1.2";
+static char *ssl_ciphers = "HIGH:!aNULL:!MD5";

 /* Ordinarily, the SSL client and server verify each other's certificates
using
  * a CA certificate.  Setting this to false disables this behavior.  (This
is a
@@ -966,6 

[ovs-dev] [PATCH v05 62/72] include/uapi/linux/openvswitch.h: use __u32 from linux/types.h

[Mikko Rapeli]

Kernel uapi header are supposed to use them. Fixes userspace compile error:

linux/openvswitch.h:583:2: error: unknown type name ‘uint32_t’

Signed-off-by: Mikko Rapeli 
---
 include/uapi/linux/openvswitch.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 645499a..54c3b4f 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -583,7 +583,7 @@ enum ovs_userspace_attr {
 #define OVS_USERSPACE_ATTR_MAX (__OVS_USERSPACE_ATTR_MAX - 1)
 
 struct ovs_action_trunc {
-   uint32_t max_len; /* Max packet size in bytes. */
+   __u32 max_len; /* Max packet size in bytes. */
 };
 
 /**
-- 
2.8.1

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

Re: [ovs-dev] [RFC PATCHv3] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

[Jesse Gross]

On Mon, Aug 22, 2016 at 6:40 AM, Sugesh Chandran
 wrote:
> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c
> index ce2582f..78ce0c9 100644
> --- a/lib/netdev-native-tnl.c
> +++ b/lib/netdev-native-tnl.c
> @@ -85,11 +85,17 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, 
> struct flow_tnl *tnl,
>
>  ovs_be32 ip_src, ip_dst;
>
> -if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
> -VLOG_WARN_RL(_rl, "ip packet has invalid checksum");
> -return NULL;
> +if(OVS_UNLIKELY(!dp_packet_ip_checksum_valid(packet))) {
> +if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
> +VLOG_WARN_RL(_rl, "ip packet has invalid checksum");
> +return NULL;
> +}
>  }
>
> +/* Reset the IP checksum offload flags if present, to avoid wrong
> + * interpretation in the further packet processing when 
> recirculated.*/
> +reset_dp_packet_ip_checksum_ol_flags(packet);
> +
>  if (ntohs(ip->ip_tot_len) > l3_size) {
>  VLOG_WARN_RL(_rl, "ip packet is truncated (IP length %d, 
> actual %d)",
>   ntohs(ip->ip_tot_len), l3_size);
> @@ -179,20 +185,26 @@ udp_extract_tnl_md(struct dp_packet *packet, struct 
> flow_tnl *tnl,
>  }
>
>  if (udp->udp_csum) {
> -uint32_t csum;
> -if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> -csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
> -} else {
> -csum = packet_csum_pseudoheader(dp_packet_l3(packet));
> -}
> -
> -csum = csum_continue(csum, udp, dp_packet_size(packet) -
> - ((const unsigned char *)udp -
> -  (const unsigned char *)dp_packet_l2(packet)));
> -if (csum_finish(csum)) {
> -return NULL;
> +if(OVS_UNLIKELY(!dp_packet_l4_checksum_valid(packet))) {
> +uint32_t csum;
> +if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> +csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
> +} else {
> +csum = packet_csum_pseudoheader(dp_packet_l3(packet));
> +}
> +
> +csum = csum_continue(csum, udp, dp_packet_size(packet) -
> + ((const unsigned char *)udp -
> +  (const unsigned char 
> *)dp_packet_l2(packet)));
> +if (csum_finish(csum)) {
> +return NULL;
> +}
>  }
>  tnl->flags |= FLOW_TNL_F_CSUM;
> +
> +/* Reset the udp checksum offload flags if present, to avoid wrong
> + * interpretation in the further packet processing when 
> recirculated.*/
> +reset_dp_packet_l4_checksum_ol_flags(packet);
>  }

Just one final comment on this - I think it's probably better to
unconditionally clear the checksum offload flags (both L3 and L4)
whenever we pop off a tunnel. Those headers will no longer exist in
the packet and therefore those flags can't have any meaning. In theory
this shouldn't make any difference compared to what you have here but
it seems a little bit more robust.

Generally, this looks good to me though. Obviously, we won't be able
to apply it until there is support for DPDK 16.11 in OVS. Will you
hold onto the patch and resubmit it at that time?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/3] OVN-Tutorial: Replace example with ASCII quotes.

[Lance Richardson]

> From: "Justin Pettit" 
> To: dev@openvswitch.org
> Sent: Thursday, August 11, 2016 2:09:12 AM
> Subject: [ovs-dev] [PATCH 1/3] OVN-Tutorial: Replace example with ASCII   
> quotes.
> 
> The "--ovn" argument for SANDBOXFLAGS used unicode quotes, which when
> copy and pasted made the command mysteriously fail.
> 
> Signed-off-by: Justin Pettit 
> ---
>  tutorial/OVN-Tutorial.md | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tutorial/OVN-Tutorial.md b/tutorial/OVN-Tutorial.md
> index be9805c..65e4ea2 100644
> --- a/tutorial/OVN-Tutorial.md
> +++ b/tutorial/OVN-Tutorial.md
> @@ -17,7 +17,7 @@ section of [Tutorial.md].
>  pass the `--ovn` flag.  For example, if running it straight from the ovs git
>  tree you would run:
>  
> -$ make sandbox SANDBOXFLAGS=”--ovn”
> +$ make sandbox SANDBOXFLAGS="--ovn"
>  
>  Running the sandbox with OVN enabled does the following additional steps to
>  the
>  environment:

Nice, I fell into this trap the first time I tried to run the ovn tutorials.

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

[ovs-dev] [PATCH v05 38/72] include/uapi/linux/openvswitch.h: use __u32 from linux/types.h

[Mikko Rapeli]

Fixes userspace compiler error:

error: unknown type name ‘uint32_t’

Signed-off-by: Mikko Rapeli 
---
 include/uapi/linux/openvswitch.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index d95a301..645499a 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -632,8 +632,8 @@ enum ovs_hash_alg {
  * @hash_basis: basis used for computing hash.
  */
 struct ovs_action_hash {
-   uint32_t  hash_alg; /* One of ovs_hash_alg. */
-   uint32_t  hash_basis;
+   __u32  hash_alg; /* One of ovs_hash_alg. */
+   __u32  hash_basis;
 };
 
 /**
-- 
2.8.1

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

[ovs-dev] [PATCH 2/3] daemon: Minor tweaking of man page fragment.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 lib/daemon.man | 2 +-
 lib/daemon.xml | 9 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/daemon.man b/lib/daemon.man
index f4e79ac..2855c2d 100644
--- a/lib/daemon.man
+++ b/lib/daemon.man
@@ -74,7 +74,7 @@ allowed, with current user or group are assumed respectively. 
Only daemons
 started by the root user accepts this argument.
 .IP
 On Linux, daemons will be granted CAP_IPC_LOCK and CAP_NET_BIND_SERVICES
-before dropping root privileges. Daemons interact with datapath,
+before dropping root privileges. Daemons that interact with a datapath,
 such as ovs-vswitchd, will be granted two additional capabilities, namely
 CAP_NET_ADMIN and CAP_NET_RAW. The capability change will apply even if
 new user is "root".
diff --git a/lib/daemon.xml b/lib/daemon.xml
index d752e99..737ae55 100644
--- a/lib/daemon.xml
+++ b/lib/daemon.xml
@@ -106,10 +106,11 @@
 
   On Linux, daemons will be granted CAP_IPC_LOCK and
   CAP_NET_BIND_SERVICES before dropping root privileges.
-  Daemons interact with datapath, such as ovs-vswitchd, will
-  be granted two additional capabilities, namely CAP_NET_ADMIN
-  and CAP_NET_RAW.  The capability change will apply even if
-  the new user is root.
+  Daemons that interact with a datapath, such as
+  ovs-vswitchd, will be granted two additional
+  capabilities, namely CAP_NET_ADMIN and
+  CAP_NET_RAW.  The capability change will apply even
+  if the new user is root.
 
 
 
-- 
1.9.1

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

[ovs-dev] [PATCH 3/3] ovn-trace: Minor cleanups.

[Justin Pettit]

Signed-off-by: Justin Pettit 
---
 ovn/utilities/ovn-trace.8.xml | 6 +++---
 ovn/utilities/ovn-trace.c | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ovn/utilities/ovn-trace.8.xml b/ovn/utilities/ovn-trace.8.xml
index 411bf1c..747e130 100644
--- a/ovn/utilities/ovn-trace.8.xml
+++ b/ovn/utilities/ovn-trace.8.xml
@@ -137,9 +137,9 @@
   
 OVN ``programs'' traces also tend to encounter long strings of logical
 flows with match expression 1 (which matches every packet)
-the single action next;.  These are uninteresting and merely
-clutter output, so ovn-trace omits them entirely even from
-detailed output.
+and the single action next;.  These are uninteresting
+and merely clutter output, so ovn-trace omits them
+entirely even from detailed output.
   
 
   
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index df2ff21..761b201 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -85,12 +85,12 @@ main(int argc, char *argv[])
 if (get_detach()) {
 if (argc != 0) {
 ovs_fatal(0, "non-option arguments not supported with --detach "
-  "(use --help for help");
+  "(use --help for help)");
 }
 } else {
 if (argc != 2) {
 ovs_fatal(0, "exactly two non-option arguments are required "
-  "(use --help for help");
+  "(use --help for help)");
 }
 }
 
-- 
1.9.1

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

[ovs-dev] [PATCH 1/3] OVN-Tutorial: Replace example with ASCII quotes.

[Justin Pettit]

The "--ovn" argument for SANDBOXFLAGS used unicode quotes, which when
copy and pasted made the command mysteriously fail.

Signed-off-by: Justin Pettit 
---
 tutorial/OVN-Tutorial.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tutorial/OVN-Tutorial.md b/tutorial/OVN-Tutorial.md
index be9805c..65e4ea2 100644
--- a/tutorial/OVN-Tutorial.md
+++ b/tutorial/OVN-Tutorial.md
@@ -17,7 +17,7 @@ section of [Tutorial.md].
 pass the `--ovn` flag.  For example, if running it straight from the ovs git
 tree you would run:
 
-$ make sandbox SANDBOXFLAGS=”--ovn”
+$ make sandbox SANDBOXFLAGS="--ovn"
 
 Running the sandbox with OVN enabled does the following additional steps to the
 environment:
-- 
1.9.1

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

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis

[Ben Pfaff]

On Mon, Aug 22, 2016 at 01:14:03PM -0400, Russell Bryant wrote:
> On Mon, Aug 22, 2016 at 12:30 PM, Ben Pfaff  wrote:
> 
> > On Tue, Aug 16, 2016 at 09:30:21AM -0400, Lance Richardson wrote:
> > > As described in ovn/TODO, these are the two main approaches that could be
> > > used to minimize the impact of a compromised chassis on the rest of an
> > > OVN OVN network:
> > >
> > >   1) Implement a role- or identity-based access control mechanism for
> > >  ovsdb-server and use it to limit ovn-controller write access to
> > >  tables in the southbound database.
> > >
> > > or
> > >
> > >   2) Disallow all write access to the southbound database by
> > ovn-controller
> > >  (as an optional mode or unconditionally) and provide alternative
> > >  mechanisms for updating the southbound database for entries that are
> > >  currently updated by ovn-controller.
> > >
> > > It is believed that option (1) would require somewhat more effort than
> > (2),
> > > and, because it would involve significant modifications to ovsdb-server,
> > > would also be more likely to add risk and burden to non-OVN users.
> > > Additionally, option (2) will likely place fewer requirements on
> > alternative
> > > databases (such as etcd), so the following implementation discussion only
> > > considers option (2).
> >
> > I've always pushed back against adding granular access control
> > mechanisms to OVSDB because I didn't believe it was likely that anything
> > that was simple enough to be in the "spirit of OVSDB" (heh) was also
> > going to be sufficient to fit a real use case.  However, if we do now
> > have specific requirements for OVN, then I'd invite descriptions of what
> > access control mechanism would be sufficient.  If it's simple and
> > general enough, then implementing it in OVSDB might totally make sense.
> >
> > I don't think that the "risk and burden" of a simple and general
> > mechanism is a real issue.
> 
> 
> I think that push back makes sense.
> 
> The proposal here was to take route #2.  The only OVSDB feature required in
> that case is to accept read-only connections, which could be on a
> per-socket basis.  This seems much simpler all around, as long as we can
> all get on board with ovn-controller as a read-only client.

I'm not actually saying we should choose #1.  I'm saying a couple of
things.  First, changing OVSDB is not a huge deal; we do it when it
makes sense.  Second, that it is possible that our specific application
here is a better place to start for OVSDB access control than a blanket
"we need access control for OVSDB" that I've heard a couple of times.

> Are you interested in looking closer at what #1 would look like, with
> details of what the access control policy would look like?

It'll probably be obvious, or close to obvious, what would be needed for
#1 once we talk through what #2 needs.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis

[Russell Bryant]

On Mon, Aug 22, 2016 at 12:30 PM, Ben Pfaff  wrote:

> On Tue, Aug 16, 2016 at 09:30:21AM -0400, Lance Richardson wrote:
> > As described in ovn/TODO, these are the two main approaches that could be
> > used to minimize the impact of a compromised chassis on the rest of an
> > OVN OVN network:
> >
> >   1) Implement a role- or identity-based access control mechanism for
> >  ovsdb-server and use it to limit ovn-controller write access to
> >  tables in the southbound database.
> >
> > or
> >
> >   2) Disallow all write access to the southbound database by
> ovn-controller
> >  (as an optional mode or unconditionally) and provide alternative
> >  mechanisms for updating the southbound database for entries that are
> >  currently updated by ovn-controller.
> >
> > It is believed that option (1) would require somewhat more effort than
> (2),
> > and, because it would involve significant modifications to ovsdb-server,
> > would also be more likely to add risk and burden to non-OVN users.
> > Additionally, option (2) will likely place fewer requirements on
> alternative
> > databases (such as etcd), so the following implementation discussion only
> > considers option (2).
>
> I've always pushed back against adding granular access control
> mechanisms to OVSDB because I didn't believe it was likely that anything
> that was simple enough to be in the "spirit of OVSDB" (heh) was also
> going to be sufficient to fit a real use case.  However, if we do now
> have specific requirements for OVN, then I'd invite descriptions of what
> access control mechanism would be sufficient.  If it's simple and
> general enough, then implementing it in OVSDB might totally make sense.
>
> I don't think that the "risk and burden" of a simple and general
> mechanism is a real issue.


I think that push back makes sense.

The proposal here was to take route #2.  The only OVSDB feature required in
that case is to accept read-only connections, which could be on a
per-socket basis.  This seems much simpler all around, as long as we can
all get on board with ovn-controller as a read-only client.

Are you interested in looking closer at what #1 would look like, with
details of what the access control policy would look like?

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

Re: [ovs-dev] [RFC PATCHv2] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

[Jesse Gross]

On Mon, Aug 22, 2016 at 6:38 AM, Chandran, Sugesh
 wrote:
>> -Original Message-
>> From: Jesse Gross [mailto:je...@kernel.org]
>> Sent: Saturday, August 20, 2016 2:07 AM
>> To: Chandran, Sugesh 
>> Cc: ovs dev 
>> Subject: Re: [RFC PATCHv2] netdev-dpdk: Enable Rx checksum offloading
>> feature on DPDK physical ports.
>> In the future, if DPDK offloads become tunnel aware some of these issues
>> can become quite complicated due to the presence of multiple checksums
>> being offloaded and how that is represented. On the Linux side there was a
>> fair amount of work put into this over the past several years, so it might be
>> worth taking a look at that for some inspiration before the DPDK interfaces
>> get locked down.
> [Sugesh] May be in the future when DPDK starts supporting checksum offload for
> inner packets, Its worth to define it as 'csum_level' with 
> CHECKSUM_UNNECESSARY than defining separate flags
> for every packet type, which is similar to what kernel is doing for tunnel 
> packet checksum offloading. The checksum reset &
> validation logic must take care of this by changing the csum_level value and 
> CHECKSUM_* flag accordingly.
>
>
> However the current DPDK driver only supports IP and L4 checksum offloading 
> using independent flags.
> Will provide the inputs when other checksum offloading features are 
> implementing in DPDK in the future.

Yes, I don't think that there is anything that can be or needs to be
done on the OVS side at this point. The main reason why I mentioned it
is to see if there is anything that we should do on the DPDK API side
now to reduce possible API incompatibilities in the future if changes
are necessary.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis

[Ben Pfaff]

On Tue, Aug 16, 2016 at 09:30:21AM -0400, Lance Richardson wrote:
> As described in ovn/TODO, these are the two main approaches that could be
> used to minimize the impact of a compromised chassis on the rest of an
> OVN OVN network:
> 
>   1) Implement a role- or identity-based access control mechanism for
>      ovsdb-server and use it to limit ovn-controller write access to
>      tables in the southbound database.
> 
> or
> 
>   2) Disallow all write access to the southbound database by ovn-controller 
>      (as an optional mode or unconditionally) and provide alternative
>      mechanisms for updating the southbound database for entries that are
>      currently updated by ovn-controller.
>      
> It is believed that option (1) would require somewhat more effort than (2),
> and, because it would involve significant modifications to ovsdb-server,
> would also be more likely to add risk and burden to non-OVN users.
> Additionally, option (2) will likely place fewer requirements on alternative
> databases (such as etcd), so the following implementation discussion only
> considers option (2).

I've always pushed back against adding granular access control
mechanisms to OVSDB because I didn't believe it was likely that anything
that was simple enough to be in the "spirit of OVSDB" (heh) was also
going to be sufficient to fit a real use case.  However, if we do now
have specific requirements for OVN, then I'd invite descriptions of what
access control mechanism would be sufficient.  If it's simple and
general enough, then implementing it in OVSDB might totally make sense.

I don't think that the "risk and burden" of a simple and general
mechanism is a real issue.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis

[Ben Pfaff]

On Mon, Aug 22, 2016 at 11:28:52AM -0400, Russell Bryant wrote:
> On Mon, Aug 22, 2016 at 11:24 AM, Ryan Moats  wrote:
> >
> > > MAC_Binding is a bit tricky - the problem here is how to deal where
> > dynamic
> > > MAC bindings need to be transferred from one chassis to another for
> > either
> > > HA or live migration scenarios. My preference here is to leave this alone
> >
> > > (i.e. allow ovn-controller to continue to write this table) and see what
> > we
> > > can apply from various anti-arp cache poisoning technologies to either
> > the IDL
> > > or ovsdb-server itself.
> > >
> > > ​The proposal here is that they wouldn't be transferred from home
> > > host to another.  Each chassis would be responsible for its own mac
> > learning.​
> >
> > That's what I'm not comfortable with...
> 
> 
> Why is that?  Isn't that how a network would typically work anyway?

Our current design for MAC_Binding is inadequate in any case and needs
to be revisited, because it does not have any provisions for
ratelimiting, query tracking, renewal, expiration, or limiting the size
of the table (see ovn/TODO for details).

Chassis-specific learning is an alternative model we've considered here.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis

[Russell Bryant]

On Mon, Aug 22, 2016 at 11:24 AM, Ryan Moats  wrote:
>
> > MAC_Binding is a bit tricky - the problem here is how to deal where
> dynamic
> > MAC bindings need to be transferred from one chassis to another for
> either
> > HA or live migration scenarios. My preference here is to leave this alone
>
> > (i.e. allow ovn-controller to continue to write this table) and see what
> we
> > can apply from various anti-arp cache poisoning technologies to either
> the IDL
> > or ovsdb-server itself.
> >
> > ​The proposal here is that they wouldn't be transferred from home
> > host to another.  Each chassis would be responsible for its own mac
> learning.​
>
> That's what I'm not comfortable with...


Why is that?  Isn't that how a network would typically work anyway?

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

Re: [ovs-dev] [PATCH 2/2] ovn/TODO: Add items proposed for 2.7 in OVN IRC meeting.

[Russell Bryant]

On Fri, Aug 19, 2016 at 11:01 AM, Ben Pfaff  wrote:

> On Fri, Aug 19, 2016 at 10:37:36AM +0530, Numan Siddique wrote:
> > On Thu, Aug 18, 2016 at 11:44 PM, Ben Pfaff  wrote:
> >
> > > Signed-off-by: Ben Pfaff 
> > > ---
> > >  ovn/TODO | 55 +++
> > >  1 file changed, 55 insertions(+)
> > >
> > > diff --git a/ovn/TODO b/ovn/TODO
> > > index b3c4831..97a5fc9 100644
> > > --- a/ovn/TODO
> > > +++ b/ovn/TODO
> > > @@ -1,5 +1,60 @@
> > >  -*- outline -*-
> > >
> > > +* Work out database for clustering or HA properly.
> > > +
> > > +* Compromised chassis mitigation.
> > > +
> > > +Possibly depends on database solution.
> > > +
> > > +* Get incremental updates in ovn-controller and ovn-northd in some
> > > +  sensible way.
> > > +
> > >
> >
> > ​May be the below can also be added which you mentioned in the IRC
> meeting
> > (11th August).
> >
> > --
> > blp regXboi: We should eventually make OVN use OpenFlow "bundles" so that
> > it transactionally replaces the flow table instead of deleting and then
> > repopulating it.
> > ​
> >
> > ​-​
>
> OK, added:
>
> --8<--cut here-->8--
>
> From: Ben Pfaff 
> Date: Fri, 19 Aug 2016 08:01:33 -0700
> Subject: [PATCH] ovn/TODO: Add items proposed for 2.7 in OVN IRC meeting.
>
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/TODO | 57 +
>  1 file changed, 57 insertions(+)
>
> diff --git a/ovn/TODO b/ovn/TODO
> index b3c4831..91600f7 100644
> --- a/ovn/TODO
> +++ b/ovn/TODO
> @@ -1,5 +1,62 @@
>  -*- outline -*-
>
> +* Work out database for clustering or HA properly.
> +
> +* Compromised chassis mitigation.
> +
> +Possibly depends on database solution.
>

I think this is already in the file under "Security".

You could replace it with this.  You could also include a link to the
latest discussion on the topic, which starts here:

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

The rest lgtm.

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

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis

[Ryan Moats]

Russell Bryant  wrote on 08/22/2016 07:56:41 AM:

> From: Russell Bryant 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: Justin Pettit , ovs dev 
> Date: 08/22/2016 07:57 AM
> Subject: Re: [ovs-dev] [RFC] ovn: minimize the impact of a
compromisedchassis
>
> On Sun, Aug 21, 2016 at 9:06 PM, Ryan Moats  wrote:
> "dev"  wrote on 08/21/2016 03:02:12 PM:
>
> > From: Justin Pettit 
> > To: Russell Bryant 
> > Cc: ovs dev 
> > Date: 08/21/2016 07:30 PM
> > Subject: Re: [ovs-dev] [RFC] ovn: minimize the impact of a
> compromised chassis
> > Sent by: "dev" 
>
> >
> >
> > > On Aug 16, 2016, at 6:52 AM, Russell Bryant 
wrote:
> > >
> > > ​Thanks for starting this discussion.​
> > >
> > > ​I do think making ovn-controller a read-only client of the database
seems
> > > like the simplest path forward.  We should pursue that until we hit a
> > > strong reason not to.
> > >
> > > One of the biggest questions remaining for me is how we deal
> with backwards
> > > compatibility.  Whatever we do here will have to account for what
happens
> > > for environments running OVN from OVS 2.6 when they upgrade.
> > >
> > > Perhaps the most straight forward way to do that is to make this new
more
> > > secure mode optional and off by default.  The downside is that
> it makes the
> > > system more complex, since it will have different modes it can work.
> >
> > I think we should avoid providing two modes if at all possible.  As
> > you mentioned, it increases the complexity, which will likely make
> > it more difficult to test thoroughly and deploy consistently.
> >
> > > An alternative would be to provide some tooling to assist with
> the upgrade:
> > >
> > > - Document the new requirements for creating Chassis and Encap rows
> > > manually
> > >
> > > - Provide an upgrade tool (via ovn-nbctl/ovn-sbctl/something-else)
that
> > > will add the "chassis" option to logical ports set based on
> where ports are
> > > currently bound in the southbound database.
> > >
> > > ​ - Provide an upgrade tool that converts the MAC_Binding table
> to whatever
> > > the new chassis local storage for that information would be.
> >
> > I think the tool approach is better.  I imagine people deploying 2.6
> > will be pretty closely tied to the community still, so walking them
> > through an upgrade shouldn't be too bad--especially if we make it
> > easy.  Obviously, we'll try to avoid the need for that again in the
future.
> >
> > --Justin

> I agree that the ovn-controller process should be limited to read-only
> for the following tables
>      SB_Global
>      Logical_Flow
>      Multicast_Group
>      Datapath_Binding
>      Address_Set
>      DHCP_Options
>      DHCPv6_Options
>
> However, I'm going to argue that the suggestions for making
ovn-controller
> read-only for
>
>  Chassis
>  Encap
>  Port_Binding
>  MAC_Binding
>
> need some more discussion as I don't think all of the suggestions to date
> are entirely feasible...
>
>
>
> First, putting the responsibility for updating the Chassis and Encap
tables
> on the CMS require (a) that the CMS have the ability to provide that
> information
> and (b) that the CMS have an OVN plugin.  I see this as moving OVN out of
the
> pure networking space (at least with respect to OpenStack) as I don't see
> Neutron getting this functionality any time soon.  I agree with Justin
that
> the right approach is to provide documentation for how to
add/remove/update
> entries via ovn-sbctl and then leave it to operators to incorporate that
> tooling into their deployment scenarios.
>

> ​I never suggested that the CMS do this.  :-)
>
> I think it should be an administrative task that is part of the
> deployment process.​

We are in agreement then...

>
> For Port_Binding, while it is possible to get that information from the
CMS,
> I worry (at least in the OpenStack case) about a potential race condition
> during instance boot - we've already seen cases during scale testingwhere
the
> current method (having the chassis update the port binding) doesn't
percolate
> through Neutron back to Nova fast enough, and using the CMS to set the
> port binding will add another half round trip of delay.  Couple that with
the
> needed modifications of ovn-controller, and I think that needs some more
> thought before we decide on how to change this.
>
> ​This is how Neutron already expects to work.  For OVN, we ignore
> the host-binding information Neutron thinks it is specifying.
> There's also already a synchronization mechanism between Nova and
> Neutron to ensure that there is not a race condition.  We have this
> implemented for OVN already (it's where we watch the 'up' column of
> Logical_Switch_Port).

I know that's how Neutron already expects to work and I know about the
synchronization mechanism - I also 

[ovs-dev] [PATCH] Documentation: Newbie guide for OVN

[Marcin Mirecki]

From 448df4a4a57ab4648c99e867596b359aa8484252 Mon Sep 17 00:00:00 2001
From: mirecki 
Date: Mon, 22 Aug 2016 16:33:32 +0200
Subject: [PATCH] Documentation: Newbie guide for OVN

Document describing the basic steps needed to get an OVN environment
running. It describes the process starting with cloning the repo, and
ending with pinging NICs connected by OVN.

Signed-off-by: Marcin Mirecki 
---
 ovn/ovn-firstSteps.xml | 279 +
 1 file changed, 279 insertions(+)
 create mode 100644 ovn/ovn-firstSteps.xml

diff --git a/ovn/ovn-firstSteps.xml b/ovn/ovn-firstSteps.xml
new file mode 100644
index 000..e372db8
--- /dev/null
+++ b/ovn/ovn-firstSteps.xml
@@ -0,0 +1,279 @@
+
+OVN - first steps
+
+This tutorial is intended to help the user to set up a working OVN 
environment>
+
+Build OVN rpm's
+
+
+Clone the repository:
+git clone https://github.com/openvswitch/ovs
+
+
+Install the following packages, they are need to build ovn:
+dnf -y install gcc make python-devel openssl-devel kernel-devel 
graphviz kernel-debug-devel autoconf automake rpm-build redhat-rpm-config 
rpm-build rpmdevtools bash-completion autoconf automake libtool PyQt4 groff 
libcap-ng-devel python-twisted-core python-zope-interface graphviz 
openssl-devel selinux-policy-devel
+
+Build the ovn rpms:
+cd ovs
+./boot.sh
+./configure
+make dist
+cp openvswitch-version.tar.gz 
$HOME/rpmbuild/SOURCES
+cd $HOME/rpmbuild/SOURCES
+tar xzf openvswitch-version.tar.gz
+cd openvswitch-version
+rpmbuild -bb rhel/openvswitch-fedora.spec  # or whichever distro you 
are running
+
+The built rpm's will reside here: 
/root/rpmbuild/RPMS/x86_64/
+
+
+Prepare environment
+
+Prepare 3 hosts (vm's). All hosts must have a nic connected to the same 
network.
+The first host will be the central server, used to host north db, south db and
+north deamon.
+The two other hosts will be used to host the ovn controllers.
+
+
+
+  CENTRAL (ovn-nb, ovn-sb, northd)
+ /   \
+/ \
+   /   \
+  / \
+ /   \
+/ \
+   /   \
+  / \
+ HOST1 (ovn-controller)  HOST2 (ovn-controller)
+
+
+Addressing scheme used in this tutorial:
+ CENTRAL - 192.168.124.9
+ HOST1 - 192.168.124.10  hostname: f10
+ HOST2 - 192.168.124.11  hostname: f11
+
+Central server installation
+
+Install the following rpms:
+dnf -y install openvswitch openvswitch-ovn-common 
openvswitch-ovn-central
+
+
+Create the north db:
+ ovsdb-tool create /etc/openvswitch/ovnnb.db 
/usr/share/openvswitch/ovn-nb.ovsschema
+Create the south db:
+ ovsdb-tool create /etc/openvswitch/ovnsb.db 
/usr/share/openvswitch/ovn-sb.ovsschema
+
+Start db:
+/usr/share/openvswitch/scripts/ovn-ctl start_ovsdb 
--db-nb-file=/etc/openvswitch/ovnnb.db 
--db-sb-file=/etc/openvswitch/ovnsb.db
+
+Start north deamon:
+/usr/share/openvswitch/scripts/ovn-ctl start_northd 
--db-nb-file=/etc/openvswitch/ovnnb.db 
--db-sb-file=/etc/openvswitch/ovnsb.db
+
+
+
+
+
+Host installation
+
+Install the following rpms:
+dnf -y install openvswitch openvswitch-ovn-common 
openvswitch-ovn-host
+
+Start openvswitch:
+systemctl start openvswitch
+
+Configure HOST1:
+ovs-vsctl set open . 
external-ids:ovn-remote=tcp:192.168.124.9:6642
+ovs-vsctl set open . external-ids:ovn-encap-type=geneve
+ovs-vsctl set open . 
external-ids:ovn-encap-ip=192.168.124.10
+
+Configure HOST2:
+ovs-vsctl set open . 
external-ids:ovn-remote=tcp:192.168.124.9:6642
+ovs-vsctl set open . external-ids:ovn-encap-type=geneve
+ovs-vsctl set open . 
external-ids:ovn-encap-ip=192.168.124.11
+
+Start ovn controller on both hosts:
+/usr/share/openvswitch/scripts/ovn-ctl start_controller
+
+
+
+Prepare virtual interfaces on hosts:
+
+Add veth pairs on the hosts.
+
+ip link add vethouter type veth peer name vethint
+vethint will be the interface added to ovs switch, while vethouter will be 
the outer interface we will use to communicate through.
+
+When successful, you should see the following output to ip 
link:
+
+HOST1:
+
+3: vethint@vethouter: BROADCAST,MULTICAST,UP,LOWER_UP mtu 1500 qdisc 
noqueue master ovs-system state UP mode DEFAULT group default qlen 1000
+link/ether be:34:f3:dd:e9:21 brd ff:ff:ff:ff:ff:ff
+4: vethouter@vethint: BROADCAST,MULTICAST,UP,LOWER_UP mtu 1500 qdisc 
noqueue state UP mode DEFAULT group default qlen 1000
+link/ether ea:d1:6a:a3:ac:c9 brd ff:ff:ff:ff:ff:ff
+
+
+HOST2:
+
+4: vethint@vethouter: BROADCAST,MULTICAST,UP,LOWER_UP mtu 1500 qdisc 
noqueue master ovs-system state UP mode DEFAULT group default qlen 1000
+link/ether 62:1c:0e:98:6f:58 brd ff:ff:ff:ff:ff:ff
+5: vethouter@vethint: BROADCAST,MULTICAST,UP,LOWER_UP mtu 1500 qdisc 
noqueue state UP mode DEFAULT group default qlen 1000
+link/ether ea:99:ae:00:dd:c3 brd ff:ff:ff:ff:ff:ff
+
+
+Create ovn connections
+
+Add logical switch and ports to north db
+Add 

Re: [ovs-dev] [PATCH V6] netdev-dpdk: Set pmd thread priority

[Bodireddy, Bhanuprakash]

>
>2016-08-18 14:20 GMT-07:00 Bodireddy, Bhanuprakash
>:
>>-Original Message-
>>From: Flavio Leitner [mailto:f...@sysclose.org]
>>Sent: Thursday, August 18, 2016 2:15 PM
>>To: Bodireddy, Bhanuprakash 
>>Cc: Daniele Di Proietto ; dev@openvswitch.org
>>Subject: Re: [ovs-dev] [PATCH V6] netdev-dpdk: Set pmd thread priority
>>
>>On Tue, Aug 16, 2016 at 02:30:04PM +, Bodireddy, Bhanuprakash wrote:
>>> >-Original Message-
>>> >From: Daniele Di Proietto [mailto:diproiet...@ovn.org]
>>> >Sent: Tuesday, August 16, 2016 1:44 AM
>>> >To: Bodireddy, Bhanuprakash 
>>> >Cc: dev@openvswitch.org; Flavio Leitner 
>>> >Subject: Re: [PATCH V6] netdev-dpdk: Set pmd thread priority
>>> >
>>> >I found a crash if apply this patch, "dpdk-lcore-mask" is not set and
>>> >"-c 0x1" is passed to "dpdk-extra".
>>> My bad, I didn't test with dpdk-extra options. I see that the crash was due
>to
>>strtol.
>>>
>>> >Also, I believe Flavio had a comment on the previous version of this
>>> >patch.  Would it be enough to use setpriority(2)?
>>> I sent out my comments in another mail. I agree to Flavio's suggestion
>>> as this seems less dangerous and is guaranteed to work even in case of
>>> misconfiguration. I tested this approach and have a concern with
>>setpriority().
>>>
>>> To apply the new nice value to the thread, thread id is needed and due
>>> to absence of glibc wrapper for gettid, I have to use syscall(SYS_gettid). I
>>want to know if this is acceptable in OVS or better way to handle this?
>>>
>>> Void ovs_numa_thread_setpriority(int nice OVS_UNUSED) { 
>>> #if defined(__linux__) && defined(SYS_gettid)
>>>      pid_t tid = syscall(SYS_gettid);
>>>      err = setpriority(PRIO_PROCESS, tid, nice);
>>>      
>>> #endif
>>> }
>>
>>I don't know a better way to implement this and it seems ovs-numa.c
>already
>>has some ifdefs specific to linux.
>>
>>Do you know if this problem happen on BSD?
>I don't know if this is a problem on BSD. I searched a bit and found BSD code
>using "syscall(SYS_thr_self, )"
>to retrieve the tid.
>
>
>The dummy ovs-numa works (and thus compile) everywhere, for pmd tests,
>but the module only works on linux.
>I think it's fine to implement something only for linux and return
>EOPNOTSUPP (only if dummy is not enabled) for everything else.
>
>I think passing 0 as thread_id to setpriority() changes the current thread
>priority, so there's probably no need for SYS_gettid.

This is helpful,  passing 0 for thread_id worked.  

>
>>
>>
>>> Without priority patch:
>>>
>>> $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e
>>pmd
>>>  22509  19   4 ovs-vswitchd
>>>  22512  19   5 lcore-slave-5
>>>  22513  19   6 lcore-slave-6
>>>  22514  19   7 lcore-slave-7
>>>  22589  19   4 revalidator37
>>>  22590  19   4 revalidator52
>>>  22591  19   4 revalidator42
>>>  22592  19   4 revalidator38
>>>  22593  19   4 revalidator39
>>>  22594  19   4 revalidator45
>>>  22595  19   4 revalidator53
>>>  22596  19   4 revalidator54
>>>  22598  19   4 pmd61                    [Default priority]
>>>
>>> With priority patch:
>>>
>>> $ ps -eLo tid,pri,psr,comm | grep -e lcore -e revalidator -e ovs-vswitchd -e
>>pmd
>>>  24879  19   4 ovs-vswitchd
>>>  24881  19   5 lcore-slave-5
>>>  24882  19   6 lcore-slave-6
>>>  24883  19   7 lcore-slave-7
>>>  24951  19   4 revalidator55
>>>  24952  19   4 revalidator37
>>>  24953  19   4 revalidator52
>>>  24954  19   4 revalidator42
>>>  24955  19   4 revalidator38
>>>  24956  19   4 revalidator39
>>>  24957  19   4 revalidator45
>>>  24958  19   4 revalidator53
>>>  24964  39   4 pmd61                  [Higher priority set]
>>
>>Looks good, so if you affinity your bash to the CPU running
>>pmd61 thread, are you able to use it?
>Yes, I tested this case and there seems to be no problem here.
>
>So there's no need to make it exclusive with dpdk-lcore-mask anymore, right?
Yes, I have sent out another version of patch which  doesn't change any of the 
existing functionality but lower the nice
Value of the pmd thread to -20.

Regards,
Bhanu Prakash.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] RETURNED MAIL: DATA FORMAT ERROR

[MAILER-DAEMON]

The original message was received at Mon, 22 Aug 2016 22:03:05 +0800
from openvswitch.org [175.243.130.243]

- The following addresses had permanent fatal errors -


- Transcript of session follows -
  while talking to openvswitch.org.:
>>> MAIL From:"MAILER-DAEMON" 
<<< 501 "MAILER-DAEMON" ... Refused



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

Re: [ovs-dev] (no subject)

[Garland Barnett]

I am sending you the bills of the goods we delivered to you in the attachment
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [RFC PATCHv3] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

[Sugesh Chandran]

Add Rx checksum offloading feature support on DPDK physical ports. By default,
the Rx checksum offloading is enabled if NIC supports. However,
the checksum offloading can be turned OFF either while adding a new DPDK
physical port to OVS or at runtime.

The rx checksum offloading can be turned off by setting the parameter to
'false'. For eg: To disable the rx checksum offloading when adding a port,

 'ovs-vsctl add-port br0 dpdk0 -- \
  set Interface dpdk0 type=dpdk options:rx-checksum-offload=false'

OR (to disable at run time after port is being added to OVS)

'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=false'

Similarly to turn ON rx checksum offloading at run time,

'ovs-vsctl set Interface dpdk0 options:rx-checksum-offload=true'

This is a RFC patch as the new checksum offload flags 'PKT_RX_L4_CKSUM_GOOD'
and 'PKT_RX_IP_CKSUM_GOOD' will be available only in DPDK 16.11 release. OVS
must compile with DPDK 16.11 release to use the checksum offloading feature.

The Tx checksum offloading support is not implemented due to the following
reasons.

1) Checksum offloading and vectorization are mutually exclusive in DPDK poll
mode driver. Vector packet processing is turned OFF when checksum offloading
is enabled which causes significant performance drop at Tx side.

2) Normally, OVS generates checksum for tunnel packets in software at the
'tunnel push' operation, where the tunnel headers are created. However
enabling Tx checksum offloading involves,

  *) Mark every packets for tx checksum offloading at 'tunnel_push' and
  recirculate.
  *) At the time of xmit, validate the same flag and instruct the NIC to do the
  checksum calculation.  In case NIC doesnt support Tx checksum offloading,
  the checksum calculation has to be done in software before sending out the
  packets.

No significant performance improvement noticed with Tx checksum offloading
due to the e overhead of additional validations + non vector packet processing.
In some test scenarios, it introduces performance drop too.

Rx checksum offloading still offers 8-9% of improvement on VxLAN tunneling
decapsulation even though the SSE vector Rx function is disabled in DPDK poll
mode driver.

Signed-off-by: Sugesh Chandran 

---
v3
- Reset the checksum offload flags in tunnel pop operation after the validation.
- Reconfigure the dpdk port with rx checksum offload only if new configuration
is different than current one.

v2
- Set Rx checksum enabled by default.
- Modified commit message, explaining the tradeoff with tx checksum offloading.
- Use dpdk mbuf checksum offload flags  instead of defining new
metadata field in OVS dp_packet.
- validate udp checksum mbuf flag only if the checksum present in the packet.
- Doc update with Rx checksum offloading feature.
---
 INSTALL.DPDK-ADVANCED.md | 18 --
 lib/dp-packet.h  | 36 
 lib/netdev-dpdk.c| 46 ++
 lib/netdev-native-tnl.c  | 42 +++---
 vswitchd/vswitch.xml | 13 +
 5 files changed, 138 insertions(+), 17 deletions(-)

diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
index 857c805..6cc42d9 100755
--- a/INSTALL.DPDK-ADVANCED.md
+++ b/INSTALL.DPDK-ADVANCED.md
@@ -14,7 +14,8 @@ OVS DPDK ADVANCED INSTALL GUIDE
 9. [Flow Control](#fc)
 10. [Pdump](#pdump)
 11. [Jumbo Frames](#jumbo)
-12. [Vsperf](#vsperf)
+12. [Rx Checksum Offload](#rx_csum)
+13. [Vsperf](#vsperf)
 
 ##  1. Overview
 
@@ -834,7 +835,20 @@ vhost ports:
  ifconfig eth1 mtu 9000
  ```
 
-##  12. Vsperf
+##  12. Rx Checksum Offload
+By default, DPDK physical ports are enabled with Rx checksum offload. Rx
+checksum offload can be configured on a DPDK physical port either when adding
+or at run time.
+
+e.g. To disable Rx checksum offload when adding a DPDK port dpdk0:
+
+`ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk 
options:rx-checksum-offload=false`
+
+e.g. To disable the Rx checksum offloading on a existing DPDK port dpdk0:
+
+`ovs-vsctl set Interface dpdk0 type=dpdk options:rx-checksum-offload=false`
+
+##  13. Vsperf
 
 Vsperf project goal is to develop vSwitch test framework that can be used to
 validate the suitability of different vSwitch implementations in a Telco 
deployment
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 7c1e637..377572b 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -592,6 +592,42 @@ dp_packet_rss_invalidate(struct dp_packet *p)
 #endif
 }
 
+static inline bool
+dp_packet_ip_checksum_valid(struct dp_packet *p)
+{
+#ifdef DPDK_NETDEV
+return p->mbuf.ol_flags & PKT_RX_IP_CKSUM_GOOD;
+#else
+return 0;
+#endif
+}
+
+static inline bool
+dp_packet_l4_checksum_valid(struct dp_packet *p)
+{
+#ifdef DPDK_NETDEV
+return p->mbuf.ol_flags & PKT_RX_L4_CKSUM_GOOD;
+#else
+return 0;
+#endif
+}
+
+static inline void
+reset_dp_packet_l4_checksum_ol_flags(struct 

Re: [ovs-dev] Mac OS X?

[Martin Segeth]

I think it is the latter. Can anyone who worked on the Mac OS port confirm?

On Sun, Aug 21, 2016 at 5:08 PM, Ben Pfaff  wrote:

> On Sun, Aug 21, 2016 at 08:12:57AM +0200, Martin Segeth wrote:
> > On Fri, Aug 19, 2016 at 10:27 PM, Lance Richardson 
> > wrote:
> >
> > > > From: "Ben Pfaff" 
> > > > To: "Martin Segeth" 
> > > > Cc: dev@openvswitch.org
> > > > Sent: Friday, August 19, 2016 3:52:05 PM
> > > > Subject: Re: [ovs-dev] Mac OS X?
> > > >
> > > > On Fri, Aug 19, 2016 at 09:44:50PM +0200, Martin Segeth wrote:
> > > > > I was trying to build OVS 2.5.0 on a Mac but get a few a errors.
> While
> > > > > researching on this topic I came across a previous thread on this
> dev
> > > > > mailing list where this was already discussed without having found
> a
> > > > > solution yet.
> > > > >
> > > > > Has anyone meanwhile managed to get it compiled on Mac OS X? If
> not, I
> > > > > guess it just does not work and would require some development
> work to
> > > get
> > > > > it ported, wouldn't it?
> > > >
> > > > I think that someone ported master to Mac OS.  Try building that.
> > >
> > > Travis does OS X builds as part of OVS CI for the master and 2.6
> branches.
> > > See https://travis-ci.org/openvswitch/ovs/builds for examples. The
> > > necessary
> > > development tools can be installed for OS X using "brew".
> > >
> > > The last time I tried, "make check" mostly passed for OS X, failures
> were
> > > generally in test cases using netdev-dummy.
> > >
> >
> > I managed to compile the master branch as suggested by Ben. The missing
> > development tools like autoconf and automake could indeed be installed
> with
> > brew.
> >
> > Though, I am unable to create a bridge interface. The userspace
> > installation readme says the datapath type is to be changed to netdev as
> > opposed to system and FreeBSD/NetBSD requires the tap driver built into
> the
> > kernel or loaded as a module.
>
> I'm not sure whether the Mac OS port is a real working port or just
> enough to compile and run the unit tests.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [RFC PATCHv2] netdev-dpdk: Enable Rx checksum offloading feature on DPDK physical ports.

[Chandran, Sugesh]

Regards
_Sugesh


> -Original Message-
> From: Jesse Gross [mailto:je...@kernel.org]
> Sent: Saturday, August 20, 2016 2:07 AM
> To: Chandran, Sugesh 
> Cc: ovs dev 
> Subject: Re: [RFC PATCHv2] netdev-dpdk: Enable Rx checksum offloading
> feature on DPDK physical ports.
> 
> On Fri, Aug 19, 2016 at 3:40 AM, Sugesh Chandran
>  wrote:
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> > e5f2cdd..113e6d8 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -1090,6 +1127,15 @@ netdev_dpdk_set_config(struct netdev
> *netdev,
> > const struct smap *args)
> >
> >  dpdk_eth_flow_ctrl_setup(dev);
> >
> > +/* Rx checksum offload configuration */
> > +/* By default the Rx checksum offload is ON */
> > +rx_chksm_ofld = smap_get_bool(args, "rx-checksum-offload", true);
> > +temp_flag = (dev->hw_ol_features &
> NETDEV_RX_CHECKSUM_OFFLOAD)
> > +!= 0;
> > +if (temp_flag != rx_chksm_ofld) {
> > +dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
> > +}
> > +dpdk_eth_checksum_offload_configure(dev);
> >  ovs_mutex_unlock(>mutex);
> 
> I think that while we are going through the trouble of checking whether the
> old flag is different from the new one, we might as well only call
> dpdk_eth_checksum_offload_configure() if there is a difference.
[Sugesh] Sure, Will move in the ' dpdk_eth_checksum_offload_configure(dev)'
inside the if statement.
> 
> > diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c index
> > ce2582f..23e987c 100644
> > --- a/lib/netdev-native-tnl.c
> > +++ b/lib/netdev-native-tnl.c
> > @@ -85,9 +85,11 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet
> > *packet, struct flow_tnl *tnl,
> >
> >  ovs_be32 ip_src, ip_dst;
> >
> > -if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
> > -VLOG_WARN_RL(_rl, "ip packet has invalid checksum");
> > -return NULL;
> > +if(OVS_UNLIKELY(!dp_packet_ip_checksum_valid(packet))) {
> > +if (csum(ip, IP_IHL(ip->ip_ihl_ver) * 4)) {
> > +VLOG_WARN_RL(_rl, "ip packet has invalid checksum");
> > +return NULL;
> > +}
> >  }
> >
> >  if (ntohs(ip->ip_tot_len) > l3_size) { @@ -179,18 +181,20 @@
> > udp_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl,
> >  }
> >
> >  if (udp->udp_csum) {
> > -uint32_t csum;
> > -if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> > -csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
> > -} else {
> > -csum = packet_csum_pseudoheader(dp_packet_l3(packet));
> > -}
> > -
> > -csum = csum_continue(csum, udp, dp_packet_size(packet) -
> > - ((const unsigned char *)udp -
> > -  (const unsigned char 
> > *)dp_packet_l2(packet)));
> > -if (csum_finish(csum)) {
> > -return NULL;
> > +if(OVS_UNLIKELY(!dp_packet_l4_checksum_valid(packet))) {
> > +uint32_t csum;
> > +if (netdev_tnl_is_header_ipv6(dp_packet_data(packet))) {
> > +csum = packet_csum_pseudoheader6(dp_packet_l3(packet));
> > +} else {
> > +csum = packet_csum_pseudoheader(dp_packet_l3(packet));
> > +}
> > +
> > +csum = csum_continue(csum, udp, dp_packet_size(packet) -
> > + ((const unsigned char *)udp -
> > +  (const unsigned char 
> > *)dp_packet_l2(packet)));
> > +if (csum_finish(csum)) {
> > +return NULL;
> > +}
> >  }
> >  tnl->flags |= FLOW_TNL_F_CSUM;
> >  }
> 
> I think the previous version of the patch cleared the offload flags after
> popping off the tunnel headers. I think that's probably a good idea here as
> well. Since we're not terminating the payload it's probably relatively rare 
> that
> we would have multiple checksums but it's theoretically possible that we
> might have a tunnel-in-tunnel situation (and I suppose that it's that we might
> want to use checksum offload for higher level services - connection
> tracking/NAT/load balancing, etc.).
> 
[Sugesh] Yes . it make sense , in fact the old patch was doing that. Will 
reset the flags after the use.

> In the future, if DPDK offloads become tunnel aware some of these issues
> can become quite complicated due to the presence of multiple checksums
> being offloaded and how that is represented. On the Linux side there was a
> fair amount of work put into this over the past several years, so it might be
> worth taking a look at that for some inspiration before the DPDK interfaces
> get locked down.
[Sugesh] May be in the future when DPDK starts supporting checksum offload for
inner packets, Its worth to define it as 'csum_level' with 

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised chassis

[Russell Bryant]

On Sun, Aug 21, 2016 at 4:02 PM, Justin Pettit  wrote:

>
> > On Aug 16, 2016, at 6:52 AM, Russell Bryant  wrote:
> >
> > ​Thanks for starting this discussion.​
> >
> > ​I do think making ovn-controller a read-only client of the database
> seems
> > like the simplest path forward.  We should pursue that until we hit a
> > strong reason not to.
> >
> > One of the biggest questions remaining for me is how we deal with
> backwards
> > compatibility.  Whatever we do here will have to account for what happens
> > for environments running OVN from OVS 2.6 when they upgrade.
> >
> > Perhaps the most straight forward way to do that is to make this new more
> > secure mode optional and off by default.  The downside is that it makes
> the
> > system more complex, since it will have different modes it can work.
>
> I think we should avoid providing two modes if at all possible.  As you
> mentioned, it increases the complexity, which will likely make it more
> difficult to test thoroughly and deploy consistently.


​Agreed.


>
> > An alternative would be to provide some tooling to assist with the
> upgrade:
> >
> > - Document the new requirements for creating Chassis and Encap rows
> > manually
> >
> > - Provide an upgrade tool (via ovn-nbctl/ovn-sbctl/something-else) that
> > will add the "chassis" option to logical ports set based on where ports
> are
> > currently bound in the southbound database.
> >
> > ​ - Provide an upgrade tool that converts the MAC_Binding table to
> whatever
> > the new chassis local storage for that information would be.
>
> I think the tool approach is better.  I imagine people deploying 2.6 will
> be pretty closely tied to the community still, so walking them through an
> upgrade shouldn't be too bad--especially if we make it easy.  Obviously,
> we'll try to avoid the need for that again in the future.
>

​Thanks for the feedback!

If we move to etcd, I imagine we'll have to provide another set of upgrade
tooling.  It would be more ideal to wait for that to solve this issue, but
I feel that's going to be too late for the primary potential user
motivating this work for me.  That's why I wanted to explore how bad it
would be to do it now.  It feels like if we can make ovn-controller
read-only, the only db-specific functionality needed is making ovsdb-server
accept connections with read-only access.  That seems like a generally
useful feature for ovsdb-server, even if we don't use it long term.

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

Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromisedchassis

[Russell Bryant]

On Sun, Aug 21, 2016 at 9:06 PM, Ryan Moats  wrote:

> "dev"  wrote on 08/21/2016 03:02:12 PM:
>
> > From: Justin Pettit 
> > To: Russell Bryant 
> > Cc: ovs dev 
> > Date: 08/21/2016 07:30 PM
> > Subject: Re: [ovs-dev] [RFC] ovn: minimize the impact of a compromised
> chassis
> > Sent by: "dev" 
>
> >
> >
> > > On Aug 16, 2016, at 6:52 AM, Russell Bryant 
> wrote:
> > >
> > > ​Thanks for starting this discussion.​
> > >
> > > ​I do think making ovn-controller a read-only client of the database
> seems
> > > like the simplest path forward.  We should pursue that until we hit a
> > > strong reason not to.
> > >
> > > One of the biggest questions remaining for me is how we deal with
> backwards
> > > compatibility.  Whatever we do here will have to account for what
> happens
> > > for environments running OVN from OVS 2.6 when they upgrade.
> > >
> > > Perhaps the most straight forward way to do that is to make this new
> more
> > > secure mode optional and off by default.  The downside is that it
> makes the
> > > system more complex, since it will have different modes it can work.
> >
> > I think we should avoid providing two modes if at all possible.  As
> > you mentioned, it increases the complexity, which will likely make
> > it more difficult to test thoroughly and deploy consistently.
> >
> > > An alternative would be to provide some tooling to assist with the
> upgrade:
> > >
> > > - Document the new requirements for creating Chassis and Encap rows
> > > manually
> > >
> > > - Provide an upgrade tool (via ovn-nbctl/ovn-sbctl/something-else)
> that
> > > will add the "chassis" option to logical ports set based on where
> ports are
> > > currently bound in the southbound database.
> > >
> > > ​ - Provide an upgrade tool that converts the MAC_Binding table to
> whatever
> > > the new chassis local storage for that information would be.
> >
> > I think the tool approach is better.  I imagine people deploying 2.6
> > will be pretty closely tied to the community still, so walking them
> > through an upgrade shouldn't be too bad--especially if we make it
> > easy.  Obviously, we'll try to avoid the need for that again in the
> future.
> >
> > --Justin
>
> I agree that the ovn-controller process should be limited to read-only
> for the following tables
>  SB_Global
>  Logical_Flow
>  Multicast_Group
>  Datapath_Binding
>  Address_Set
>  DHCP_Options
>  DHCPv6_Options
>
> However, I'm going to argue that the suggestions for making ovn-controller
> read-only for
>
>  Chassis
>  Encap
>  Port_Binding
>  MAC_Binding
>
> need some more discussion as I don't think all of the suggestions to date
> are entirely feasible...
>


>
> First, putting the responsibility for updating the Chassis and Encap tables
> on the CMS require (a) that the CMS have the ability to provide that
> information
> and (b) that the CMS have an OVN plugin.  I see this as moving OVN out of
> the
> pure networking space (at least with respect to OpenStack) as I don't see
> Neutron getting this functionality any time soon.  I agree with Justin that
> the right approach is to provide documentation for how to add/remove/update
> entries via ovn-sbctl and then leave it to operators to incorporate that
> tooling into their deployment scenarios.
>

​I never suggested that the CMS do this.  :-)

I think it should be an administrative task that is part of the deployment
process.​


>
>
> For Port_Binding, while it is possible to get that information from the
> CMS,
> I worry (at least in the OpenStack case) about a potential race condition
> during instance boot - we've already seen cases during scale testing where
> the
> current method (having the chassis update the port binding) doesn't
> percolate
> through Neutron back to Nova fast enough, and using the CMS to set the
> port binding will add another half round trip of delay.  Couple that with
> the
> needed modifications of ovn-controller, and I think that needs some more
> thought before we decide on how to change this.
>

​This is how Neutron already expects to work.  For OVN, we ignore the
host-binding information Neutron thinks it is specifying.  There's also
already a synchronization mechanism between Nova and Neutron to ensure that
there is not a race condition.  We have this implemented for OVN already
(it's where we watch the 'up' column of Logical_Switch_Port).

MAC_Binding is a bit tricky - the problem here is how to deal where dynamic
> MAC bindings need to be transferred from one chassis to another for either
> HA or live migration scenarios. My preference here is to leave this alone
> (i.e. allow ovn-controller to continue to write this table) and see what we
> can apply from various anti-arp cache poisoning technologies to either the
> IDL
> or ovsdb-server itself.
>
> ​The proposal here is that they 

[ovs-dev] [CudaMailTagged] Re: [PATCH] ovn-controller: add quiet mode

[Ryan Moats]

Hui Kang/Watson/IBM wrote on 08/21/2016 10:23:21 PM:

> From: Hui Kang/Watson/IBM
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 08/21/2016 10:23 PM
> Subject: Re: [ovs-dev] [PATCH] ovn-controller: add quiet mode
>
> I did observe that the previous incremental patch improves the ova-
> controller's CPU utilization a lot.
>
> I will test these two patch to ensure that the improvement will not
> disappear. Thanks.
>
> - Hui

Hui-

Please also consider v2 of the "add quiet mode" patch for this. I
put v1 in a four node OpenStack cloud and found that
ovn-controller didn't quiesce once I had a patch port on br-int.
v2 addresses this issue, but I believe there is still an issue once
I have a logical router connecting two logical switches.  While I'm
looking at this myself today, an independent set of eyes would be
appreciated.

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

[ovs-dev] [PATCH V7] netdev-dpdk: Increase pmd thread priority

[Bhanuprakash Bodireddy]

Increase the DPDK pmd thread scheduling priority by lowering the nice
value. This will advise the kernel scheduler to prioritize pmd thread
over other processes.

Signed-off-by: Bhanuprakash Bodireddy 
---
v6->v7:
* Remove realtime scheduling policy logic.
* Increase pmd thread scheduling priority by lowering nice value to -20.
* Update doc accordingly.

v5->v6:
* Prohibit spawning pmd thread on the lowest core in dpdk-lcore-mask if
  lcore-mask and pmd-mask affinity are identical.
* Updated Note section in INSTALL.DPDK-ADVANCED doc.
* Tested below cases to verify system stability with pmd priority patch

   dpdk-lcore-mask | pmd-cpu-mask | Comment
1.   Not set   |  Not set | control threads affinity: 0-27
pmd thread: core 0
2. 1   |  1   | pmd thread isn't spawned and warning
logged in logfile.
3. 1   |  c   |  
4. F0  |  F0  | control threads pinned to core 4.
3 pmd threads created on core 5,6,7 but 4.

v4->v5:
* Reword Note section in DPDK-ADVANCED.md

v3->v4:
* Document update
* Use ovs_strerror for reporting errors in lib-numa.c

v2->v3:
* Move set_priority() function to lib/ovs-numa.c
* Apply realtime scheduling policy and priority to pmd thread only if
  pmd-cpu-mask is passed.
* Update INSTALL.DPDK-ADVANCED.

v1->v2:
* Removed #ifdef and introduced dummy function "pmd_thread_setpriority"
  in netdev-dpdk.h
* Rebase

 INSTALL.DPDK-ADVANCED.md |  9 +++--
 lib/dpif-netdev.c|  4 
 lib/ovs-numa.c   | 19 +++
 lib/ovs-numa.h   |  1 +
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/INSTALL.DPDK-ADVANCED.md b/INSTALL.DPDK-ADVANCED.md
index 857c805..2b0045d 100755
--- a/INSTALL.DPDK-ADVANCED.md
+++ b/INSTALL.DPDK-ADVANCED.md
@@ -208,8 +208,9 @@ needs to be affinitized accordingly.
 pmd thread is CPU bound, and needs to be affinitized to isolated
 cores for optimum performance.
 
-By setting a bit in the mask, a pmd thread is created and pinned
-to the corresponding CPU core. e.g. to run a pmd thread on core 2
+By setting a bit in the mask, a pmd thread is created, pinned
+to the corresponding CPU core and the nice value set to '-20'.
+e.g. to pin a pmd thread on core 2
 
 `ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=4`
 
@@ -249,6 +250,10 @@ needs to be affinitized accordingly.
 
   NIC port0 <-> OVS <-> VM <-> OVS <-> NIC port 1
 
+  Note: It is recommended that the OVS control thread and pmd thread
+  shouldn't be pinned to same core i.e 'dpdk-lcore-mask' and 'pmd-cpu-mask'
+  cpu mask settings should be non-overlapping.
+
 ### 4.3 DPDK physical port Rx Queues
 
   `ovs-vsctl set Interface  options:n_rxq=`
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ecc7cea..119e66e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3100,6 +3100,10 @@ pmd_thread_main(void *f_)
 ovs_numa_thread_setaffinity_core(pmd->core_id);
 dpdk_set_lcore_id(pmd->core_id);
 poll_cnt = pmd_load_queues_and_ports(pmd, _list);
+
+/* Set pmd thread's nice value to -20 */
+#define MIN_NICE -20
+ovs_numa_thread_setpriority(MIN_NICE);
 reload:
 emc_cache_init(>flow_cache);
 
diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
index c8173e0..f5daa41 100644
--- a/lib/ovs-numa.c
+++ b/lib/ovs-numa.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #endif /* __linux__ */
 
@@ -613,3 +614,21 @@ int ovs_numa_thread_setaffinity_core(unsigned core_id 
OVS_UNUSED)
 return EOPNOTSUPP;
 #endif /* __linux__ */
 }
+
+void
+ovs_numa_thread_setpriority(int nice OVS_UNUSED)
+{
+if (dummy_numa) {
+return;
+}
+
+#ifndef _WIN32
+int err;
+err = setpriority(PRIO_PROCESS, 0, nice);
+if (err) {
+VLOG_ERR("Thread priority error %s",ovs_strerror(err));
+}
+#else
+return EOPNOTSUPP;
+#endif
+}
diff --git a/lib/ovs-numa.h b/lib/ovs-numa.h
index be836b2..3ce13c7 100644
--- a/lib/ovs-numa.h
+++ b/lib/ovs-numa.h
@@ -56,6 +56,7 @@ void ovs_numa_unpin_core(unsigned core_id);
 struct ovs_numa_dump *ovs_numa_dump_cores_on_numa(int numa_id);
 void ovs_numa_dump_destroy(struct ovs_numa_dump *);
 int ovs_numa_thread_setaffinity_core(unsigned core_id);
+void ovs_numa_thread_setpriority(int nice);
 
 #define FOR_EACH_CORE_ON_NUMA(ITER, DUMP)\
 LIST_FOR_EACH((ITER), list_node, &(DUMP)->dump)
-- 
2.4.11

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

[ovs-dev] Today's fax

[Dominique]

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

[ovs-dev] OVN:Does ovn already support floating ip？

[huangdenghui]

发自网易邮箱手机版
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net-next v11 5/6] openvswitch: add layer 3 flow/port support

[Simon Horman]

On Wed, Aug 10, 2016 at 10:17:30AM -0700, Joe Stringer wrote:
> On 10 August 2016 at 03:20, Simon Horman  wrote:
> > On Tue, Aug 09, 2016 at 08:47:40AM -0700, pravin shelar wrote:
> >> On Mon, Aug 8, 2016 at 8:17 AM, Simon Horman  
> >> wrote:
> >> > Light testing seems to indicate that it works for GSO skbs
> >> > received over both L3 and L2 GRE tunnels by OvS with both
> >> > IP-in-MPLS and IP (without MPLS) payloads.
> >> >
> >>
> >> Thanks for testing it. Can you also add those tests to OVS kmod test suite?
> >> ..
> >
> > Sure, I will look into doing that.
> > Am I correct in thinking Joe Stringer is the best person to contact if
> > I run into trouble there?
> 
> Sure. The basics of running the tests is documented here:
> https://github.com/openvswitch/ovs/blob/master/INSTALL.md#datapath-testing
> 
> You should be able to get a good feel for how to add tests by perusing
> the commits to tests/system-{traffic,kmod-macros}.at in the OVS source
> tree.

Thanks Joe,

it took me a while but I think that I have something working
against the head branch of the OVS tree. I'd value opinions
on the direction I have taken.

Subject: [PATCH] system-traffic: Exercise GSO

Exercise GSO for: unencapsulated; MPLS; GRE; and MPLS in GRE.

There is scope to extend this testing to other encapsulation formats
if desired.

This is motivated by a desire to test GRE and MPLS encapsulation in
the context of L3/VPN (MPLS over non-TEB GRE work). That is not
tested here but tests for those cases would ideally be based on those in
this patch.

---
 tests/system-common-macros.at |  36 +--
 tests/system-kmod-macros.at   |  22 +
 tests/system-traffic.at   | 225 +-
 3 files changed, 274 insertions(+), 9 deletions(-)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 4ffc3822a4d3..a201cf8ce100 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -56,7 +56,7 @@ m4_define([ADD_INT],
 ]
 )
 
-# ADD_VETH([port], [namespace], [ovs-br], [ip_addr] [mac_addr [gateway]])
+# ADD_VETH([port], [namespace], [ovs-br], [ip_addr] [mac_addr [gateway 
[ofport]]])
 #
 # Add a pair of veth ports. 'port' will be added to name space 'namespace',
 # and "ovs-'port'" will be added to ovs bridge 'ovs-br'.
@@ -64,8 +64,8 @@ m4_define([ADD_INT],
 # The 'port' in 'namespace' will be brought up with static IP address
 # with 'ip_addr' in CIDR notation.
 #
-# Optionally, one can specify the 'mac_addr' for 'port' and the default
-# 'gateway'.
+# Optionally, one can specify the 'mac_addr' for 'port', the default
+# 'gateway' and the 'ofport' number.
 #
 # The existing 'port' or 'ovs-port' will be removed before new ones are added.
 #
@@ -74,8 +74,14 @@ m4_define([ADD_VETH],
   CONFIGURE_VETH_OFFLOADS([$1])
   AT_CHECK([ip link set $1 netns $2])
   AT_CHECK([ip link set dev ovs-$1 up])
-  AT_CHECK([ovs-vsctl add-port $3 ovs-$1 -- \
-set interface ovs-$1 external-ids:iface-id="$1"])
+  if test -n "$7"; then
+AT_CHECK([ovs-vsctl add-port $3 ovs-$1 -- \
+  set interface ovs-$1 external-ids:iface-id="$1" \
+  ofport_request=$7])
+  else
+AT_CHECK([ovs-vsctl add-port $3 ovs-$1 -- \
+  set interface ovs-$1 external-ids:iface-id="$1"])
+  fi
   NS_CHECK_EXEC([$2], [ip addr add $4 dev $1])
   NS_CHECK_EXEC([$2], [ip link set dev $1 up])
   if test -n "$5"; then
@@ -99,7 +105,7 @@ m4_define([ADD_VLAN],
 ]
 )
 
-# ADD_OVS_TUNNEL([type], [bridge], [port], [remote-addr], [overlay-addr])
+# ADD_OVS_TUNNEL([type], [bridge], [port], [remote-addr], [overlay-addr 
[ofport]])
 #
 # Add an ovs-based tunnel device in the root namespace, with name 'port' and
 # type 'type'. The tunnel device will be configured as point-to-point with the
@@ -107,9 +113,17 @@ m4_define([ADD_VLAN],
 #
 # 'port will be configured with the address 'overlay-addr'.
 #
+# Optionally one can specify the 'ofport' number
+#
 m4_define([ADD_OVS_TUNNEL],
-   [AT_CHECK([ovs-vsctl add-port $2 $3 -- \
-  set int $3 type=$1 options:remote_ip=$4])
+   [if test -n "$6"; then
+  AT_CHECK([ovs-vsctl add-port $2 $3 -- \
+set int $3 type=$1 options:remote_ip=$4 \
+   ofport_request=$6])
+else
+  AT_CHECK([ovs-vsctl add-port $2 $3 -- \
+set int $3 type=$1 options:remote_ip=$4])
+fi
 AT_CHECK([ip addr add dev $2 $5])
 AT_CHECK([ip link set dev $2 up])
 AT_CHECK([ip link set dev $2 mtu 1450])
@@ -143,6 +157,12 @@ m4_define([ADD_NATIVE_TUNNEL],
 #
 m4_define([FORMAT_PING], [grep "transmitted" | sed 's/time.*ms$/time 0ms/'])
 
+# FORMAT_DD([])
+#
+# Strip variant pieces from dd output so the output can be reliably compared.
+#
+m4_define([FORMAT_DD], [sed 's/copied,.*$/copied, .../'])
+
 # FORMAT_CT([ip-addr])
 #
 # Strip content from the piped input which 

[ovs-dev] Today's fax

[Jodi]

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

Re: [ovs-dev] [PATCH v1] Basic GTP-U tunnel implementation in ovs

[Ashish Kurian]

Dear Members,

I tried to implement the patch using the patch command and I am not able to
complete the patching. I am new to this and I could use some help from you
members on how I can patch this patch.

When I try to implement, I am getting the message on my terminal that "File
to patch:"

I do not know how to proceed from here.

Best Regards,
Ashish Kurian
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] status

[The Post Office]

Dear user of openvswitch.org, mail server administrator of openvswitch.org 
would like to let you know that.

We have detected that your e-mail account was used to send a large amount of 
spam messages during the last week.
Most likely your computer was infected and now runs a trojaned proxy server.

We recommend you to follow our instructions in order to keep your computer safe.

Sincerely yours,
openvswitch.org support team.

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

Re: [ovs-dev] [PATCH v2] ovn: Delete stale MAC_Binding records

[Liran Schour]

Amitabha Biswas  wrote on 20/08/2016 08:14:36 AM:

> We have seen our Cloud deployment and usage come to a halt after 
> deleting a Logical Router, due the problem being addressed below. 
> The MAC_Bindings have a strong reference to the Datapath_Binding. 
> However the MAC_Bindings are never deleted anywhere, and when the 
> Datapath (associated with a MAC_Binding) is deleted, the ovsdb-
> server returns Referential Integrity Violation. This prevents newer 
> operations initiated from the CMS (in our case OpenStack) from being
> committed to the Southbound DB (the northd keeps getting the 
> violation for newer commits).
> 
> Another way to avoid the Referential Integrity Violation in this 
> case is to make the datapath Column have a weak reference (default 
> is strong) to the Datapath_Binding row and allow the column to be 
> empty (min 0). We can probably add that on top of the current patch.
> I?m making that assumption that it won?t affect the conditional 
> monitoring patch (791a7747427310f6a09c7b2f57a99d65338dfb45) which 
> introduced the Data_Binding column strong reference.
> 

Yes it will work but I think that it is better to delete a MAC_Binding row 
when the logical port is deleted. There is no sense on keeping a 
MAC_Binding row of a deleted logical port.

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