Re: [ovs-dev] [PATCH] netdev-provider: Apply batch object to netdev provider.

[William Tu]

1. We can use batch API:
Example1:
-for (i = 0; i < cnt; i++) {
-dp_packet_delete(pkts[i]);
-}
+dp_packet_delete_batch(batch, may_steal);

Example2:
-for (i = 0; i < cnt; i++) {
-int cutlen = dp_packet_get_cutlen(pkts[i]);
-
-dp_packet_set_size(pkts[i], dp_packet_size(pkts[i]) - cutlen);
-dp_packet_reset_cutlen(pkts[i]);
-}
-__netdev_dpdk_vhost_send(netdev, qid, pkts, cnt, may_steal);
+dp_packet_batch_apply_cutlen(batch);
+__netdev_dpdk_vhost_send(netdev, qid, batch, may_steal);

2. Certain batch-level attribute can be passed into netdev provider to
avoid loop. For example:
Commit aaca4fe0ce9 introduces 'trunc' in dp_packet_batch.
if batch->trunc == false, then the entire batch of packet can skip the
truncate action, we don't need to loop into each packet and check.

Regards,
William

On Fri, Jun 24, 2016 at 10:35 PM, Ben Pfaff  wrote:
> On Fri, Jun 24, 2016 at 04:11:47PM -0700, William Tu wrote:
>> This patch applies the packet batch object to the netdev providers,
>> including dummy, Linux, BSD, and DPDK.
>>
>> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/140135888
>> Signed-off-by: William Tu 
>
> What's the benefit?
>
> I haven't read the patch yet.
>
> Thanks,
>
> Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] netdev-provider: Apply batch object to netdev provider.

[Ben Pfaff]

On Fri, Jun 24, 2016 at 04:11:47PM -0700, William Tu wrote:
> This patch applies the packet batch object to the netdev providers,
> including dummy, Linux, BSD, and DPDK.
> 
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/140135888
> Signed-off-by: William Tu 

What's the benefit?

I haven't read the patch yet.

Thanks,

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

Re: [ovs-dev] [PATCH] FAQ: Describe how to use "learn" as a primitive.

[Ben Pfaff]

On Sun, Jun 19, 2016 at 02:42:29PM -0500, Ryan Moats wrote:
> "dev"  wrote on 06/19/2016 12:24:11 PM:
> 
> > From: Ben Pfaff 
> > To: dev@openvswitch.org
> > Cc: Ben Pfaff 
> > Date: 06/19/2016 12:24 PM
> > Subject: [ovs-dev] [PATCH] FAQ: Describe how to use "learn" as a
> primitive.
> > Sent by: "dev" 
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> >  FAQ.md | 42 ++
> >  1 file changed, 42 insertions(+)
> >
> > diff --git a/FAQ.md b/FAQ.md
> > index df6f225..2babb8f 100644
> > --- a/FAQ.md
> > +++ b/FAQ.md
> > @@ -2029,6 +2029,47 @@ A: OpenFlow actions are executed in the order
> > specified.  Thus, the
> >
> > ovs-ofctl add-flow br0
> dl_vlan=123,actions=mod_vlan_vid:456,output:1
> >
> > +### Q: The "learn" action can't learn the action I want, can you improve
> it?
> > +
> > +A: By itself, the "learn" action can only put two kinds of actions
> > +   into the flows that it creates: "load" and "output" actions.  If
> > +   "learn" is used in isolation, these are severe limits.
> > +
> > +   Howver, "learn" is not meant to be used in isolation.  It is a
> 
> Typo: However

Thanks, fixed.

> > +   primitive meant to be used together with other Open vSwitch
> > +   features to accomplish a task.  Its existing features are enough to
> 
> Typo: I think this "Its" should be "It's" because it is a possessive.

It should be "its" because it's a possessive, see
e.g. http://grammarist.com/spelling/its-its/

The best form of it[']s is the It's-It, which makes me possessive
despite the apostrophe.  http://tinyurl.com/lw78q9r

> Once the two potential typos are fixed...
> 
> Acked-by: Ryan Moats 

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

[ovs-dev] Cooperation with the great company

[dev]

Hello!

We are looking for employees working remotely.

My name is Lourdes, am the personnel manager of a large International company.
Most of the work you can do from home, that is, at a distance.

Salary is $2500-$5000.

If you are interested in this offer, please visit 
Our Site

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

[ovs-dev] Career growth

[dev]

Hello!

We are looking for employees working remotely.

My name is Sonja, am the personnel manager of a large International company.
Most of the work you can do from home, that is, at a distance.

Salary is $2500-$5000.

If you are interested in this offer, please visit 
Our Site

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

Re: [ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Ben Pfaff]

Thanks!

On Sat, Jun 25, 2016 at 04:34:49AM +, Daniel Ye wrote:
> OK, I got it and I will handle this.
> 
> Bests,
> Daniel
> 
> > On Jun 25, 2016, at 12:22 PM, Ben Pfaff  wrote:
> > 
> > On Sat, Jun 25, 2016 at 04:12:04AM +, Daniel Ye wrote:
> >> It’s because of the "packet send error" statistic. The test of checking 
> >> IPFIX statistics set collector as “127.0.0.1:4739”.
> >> Test code:
> >> -
> >> dnl Sample every packet using bridge-based sampling.
> >> AT_CHECK([ovs-vsctl -- set bridge br0 ipfix=@fix -- \
> >>--id=@fix create ipfix targets=\"127.0.0.1:4739\" \
> >>  sampling=1], [0], [ignore])
> >> ——
> >> 
> >> We expect “ tx errs=12” because we don’t listen on port 4739 on local host.
> >> Test code:
> >> -
> >> AT_CHECK([ovs-ofctl dump-ipfix-bridge br0], [0], [dnl
> >> NXST_IPFIX_BRIDGE reply (xid=0x2):
> >>  bridge ipfix: flows=20, current flows=0, sampled pkts=20, ipv4 ok=0, ipv6 
> >> ok=0, tx pkts=12
> >>pkts errs=20, ipv4 errs=20, ipv6 errs=0, tx errs=12
> >> ])
> >> ——
> >> 
> >> As I talked with Wenyu, she has listened on port 4739 for testing. If port 
> >> 4739 on the local host is listened, there will
> >> be no "tx errs”.
> >> 
> >> Should we remove "tx errs” check?
> > 
> > OK, that's a bug in the test then.  The tests shouldn't depend on or
> > require listening on any fixed port numbers.  A lot of tests use the
> > PARSE_LISTENING_PORT macro to avoid the need to listen on a fixed port.
> > Here's the definition from ofproto-macros.at:
> > 
> ># PARSE_LISTENING_PORT LOGFILE VARIABLE
> >#
> ># Parses the TCP or SSL port on which a server is listening from
> ># LOGFILE, given that the server was told to listen on a kernel-chosen
> ># port, and assigns the port number to shell VARIABLE.  You should
> ># specify the listening remote as ptcp:0:127.0.0.1 or
> ># pssl:0:127.0.0.1, or the equivalent with [::1] instead of 127.0.0.1.
> >#
> ># Here's an example of how to use this with ovsdb-server:
> >#
> >#ovsdb-server --log-file --remote=ptcp:0:127.0.0.1 ...
> >#PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
> >## Now $TCP_PORT holds the listening port.
> >m4_define([PARSE_LISTENING_PORT],
> >[OVS_WAIT_UNTIL([$2=`sed -n 's/.*0:.*: listening on port 
> > \([[0-9]]*\)$/\1/p' "$1"` && test X != X"[$]$2"])])
> > 
> >start_daemon () {
> >"$@" -vconsole:off --detach --no-chdir --pidfile --log-file
> >pid=`cat "$OVS_RUNDIR"/$1.pid`
> >on_exit "kill $pid"
> >}
> > 
> > You can find several examples of its use in the tests directory.
> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] netdev-linux: Do not log a warning if the device is down.

[Ben Pfaff]

On Fri, Jun 10, 2016 at 03:52:16PM -0700, Daniele Di Proietto wrote:
> In the userspace datapath we use tap devices as internal netdev.  The
> datapath doesn't consider whether a device is up or down before sending
> to it, and so far this hasn't been a problem.
> 
> Since Linux upstream commit 1bd4978a88ac("tun: honor IFF_UP in
> tun_get_user()"), included in 4.4, writing to a tap device that is not
> up sets errno to EIO.  This commit avoids printing a warning in this
> case.
> 
> This fixes a failures in the system-userspace-testsuites.
> 
> Reported-by: Joe Stringer 
> Signed-off-by: Daniele Di Proietto 

Acked-by: Ben Pfaff 

However, the logic is getting a little nonobvious here in my opinion.
Here's a version that I think is equivalent (I have not tested it) and
seems to me easier to understand:

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index ef11d12..1cc0a5f 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1223,15 +1223,20 @@ netdev_linux_send(struct netdev *netdev_, int qid 
OVS_UNUSED,
 }
 
 if (retval < 0) {
-/* The Linux AF_PACKET implementation never blocks waiting for room
- * for packets, instead returning ENOBUFS.  Translate this into
- * EAGAIN for the caller. */
-error = errno == ENOBUFS ? EAGAIN : errno;
-if (error == EINTR) {
-/* continue without incrementing 'i', i.e. retry this packet */
+if (errno == EINTR) {
+/* The send was interrupted by a signal.  Retry the packet by
+ * continuing without incrementing 'i'. */
 continue;
+} else if (errno == EIO && is_tap_netdev(netdev_)) {
+/* The Linux tap driver returns EIO if the device is not up.
+ * From the OVS side this is not an error, so ignore it. */
+} else {
+/* The Linux AF_PACKET implementation never blocks waiting for
+ * room for packets, instead returning ENOBUFS.  Translate this
+ * into EAGAIN for the caller. */
+error = errno == ENOBUFS ? EAGAIN : errno;
+break;
 }
-break;
 } else if (retval != size) {
 VLOG_WARN_RL(, "sent partial Ethernet packet (%"PRIuSIZE" bytes"
   " of %"PRIuSIZE") on %s", retval, size,

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

Re: [ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Daniel Ye]

OK, I got it and I will handle this.

Bests,
Daniel

> On Jun 25, 2016, at 12:22 PM, Ben Pfaff  wrote:
> 
> On Sat, Jun 25, 2016 at 04:12:04AM +, Daniel Ye wrote:
>> It’s because of the "packet send error" statistic. The test of checking 
>> IPFIX statistics set collector as “127.0.0.1:4739”.
>> Test code:
>> -
>> dnl Sample every packet using bridge-based sampling.
>> AT_CHECK([ovs-vsctl -- set bridge br0 ipfix=@fix -- \
>>--id=@fix create ipfix targets=\"127.0.0.1:4739\" \
>>  sampling=1], [0], [ignore])
>> ——
>> 
>> We expect “ tx errs=12” because we don’t listen on port 4739 on local host.
>> Test code:
>> -
>> AT_CHECK([ovs-ofctl dump-ipfix-bridge br0], [0], [dnl
>> NXST_IPFIX_BRIDGE reply (xid=0x2):
>>  bridge ipfix: flows=20, current flows=0, sampled pkts=20, ipv4 ok=0, ipv6 
>> ok=0, tx pkts=12
>>pkts errs=20, ipv4 errs=20, ipv6 errs=0, tx errs=12
>> ])
>> ——
>> 
>> As I talked with Wenyu, she has listened on port 4739 for testing. If port 
>> 4739 on the local host is listened, there will
>> be no "tx errs”.
>> 
>> Should we remove "tx errs” check?
> 
> OK, that's a bug in the test then.  The tests shouldn't depend on or
> require listening on any fixed port numbers.  A lot of tests use the
> PARSE_LISTENING_PORT macro to avoid the need to listen on a fixed port.
> Here's the definition from ofproto-macros.at:
> 
># PARSE_LISTENING_PORT LOGFILE VARIABLE
>#
># Parses the TCP or SSL port on which a server is listening from
># LOGFILE, given that the server was told to listen on a kernel-chosen
># port, and assigns the port number to shell VARIABLE.  You should
># specify the listening remote as ptcp:0:127.0.0.1 or
># pssl:0:127.0.0.1, or the equivalent with [::1] instead of 127.0.0.1.
>#
># Here's an example of how to use this with ovsdb-server:
>#
>#ovsdb-server --log-file --remote=ptcp:0:127.0.0.1 ...
>#PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>## Now $TCP_PORT holds the listening port.
>m4_define([PARSE_LISTENING_PORT],
>[OVS_WAIT_UNTIL([$2=`sed -n 's/.*0:.*: listening on port 
> \([[0-9]]*\)$/\1/p' "$1"` && test X != X"[$]$2"])])
> 
>start_daemon () {
>"$@" -vconsole:off --detach --no-chdir --pidfile --log-file
>pid=`cat "$OVS_RUNDIR"/$1.pid`
>on_exit "kill $pid"
>}
> 
> You can find several examples of its use in the tests directory.

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

[ovs-dev] [PATCH v2] sset: New function sset_from_delimited_string().

[Ben Pfaff]

This simplifies code in a couple of places.

Signed-off-by: Ben Pfaff 
---
v1->v2: Fix compile errors.

 lib/sset.c| 21 -
 lib/sset.h|  3 +++
 ovn/utilities/ovn-sbctl.c | 10 ++
 ovsdb/replication.c   | 12 +---
 4 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/lib/sset.c b/lib/sset.c
index 4fd3fae..be29cc7 100644
--- a/lib/sset.c
+++ b/lib/sset.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2011, 2012, 2013, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -99,6 +99,25 @@ sset_moved(struct sset *set)
 hmap_moved(>map);
 }
 
+/* Initializes 'set' with substrings of 's' that are delimited by any of the
+ * characters in 'delimiters'.  For example,
+ * sset_from_delimited_string(, "a b,c", " ,");
+ * initializes 'set' with three strings "a", "b", and "c". */
+void
+sset_from_delimited_string(struct sset *set, const char *s_,
+   const char *delimiters)
+{
+sset_init(set);
+
+char *s = xstrdup(s_);
+char *token, *save_ptr = NULL;
+for (token = strtok_r(s, delimiters, _ptr); token != NULL;
+ token = strtok_r(NULL, delimiters, _ptr)) {
+sset_add(set, token);
+}
+free(s);
+}
+
 /* Returns true if 'set' contains no strings, false if it contains at least one
  * string. */
 bool
diff --git a/lib/sset.h b/lib/sset.h
index 9c2f703..c3b5e97 100644
--- a/lib/sset.h
+++ b/lib/sset.h
@@ -43,6 +43,9 @@ void sset_clone(struct sset *, const struct sset *);
 void sset_swap(struct sset *, struct sset *);
 void sset_moved(struct sset *);
 
+void sset_from_delimited_string(struct sset *, const char *s,
+const char *delimiters);
+
 /* Count. */
 bool sset_is_empty(const struct sset *);
 size_t sset_count(const struct sset *);
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index c0ee518..40e1797 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -548,14 +548,8 @@ cmd_chassis_add(struct ctl_context *ctx)
 check_conflicts(sbctl_ctx, ch_name,
 xasprintf("cannot create a chassis named %s", ch_name));
 
-char *tokstr = xstrdup(encap_types);
-char *token, *save_ptr = NULL;
-struct sset encap_set = SSET_INITIALIZER(_set);
-for (token = strtok_r(tokstr, ",", _ptr); token != NULL;
- token = strtok_r(NULL, ",", _ptr)) {
-sset_add(_set, token);
-}
-free(tokstr);
+struct sset encap_set;
+sset_from_delimited_string(_set, encap_types, ",");
 
 size_t n_encaps = sset_count(_set);
 struct sbrec_encap **encaps = xmalloc(n_encaps * sizeof *encaps);
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 1fa0f25..aa6b9e2 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -121,19 +121,9 @@ set_remote_ovsdb_server(const char *remote_server)
 void
 set_tables_blacklist(const char *blacklist)
 {
-char *save_ptr = NULL;
-char *blacklist_item;
-
 replication_init();
-
 if (blacklist) {
-char *t_blacklist = xstrdup(blacklist);
-for (blacklist_item = strtok_r(t_blacklist, ",", _ptr);
- blacklist_item != NULL;
- blacklist_item = strtok_r(NULL, ",", _ptr)) {
-sset_add(_blacklist, blacklist_item);
-}
-free(t_blacklist);
+sset_from_delimited_string(_blacklist, blacklist, ",");
 }
 }
 
-- 
2.1.3

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

Re: [ovs-dev] [PATCH] sset: New function sset_from_delimited_string().

[Ben Pfaff]

On Fri, Jun 24, 2016 at 05:16:03PM -0700, Ben Pfaff wrote:
> This simplifies code in a couple of places.
> 
> Signed-off-by: Ben Pfaff 

Oops, this doesn't build.

I'll send v2.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] util: New function nullable_xstrdup().

[Ben Pfaff]

It's a pretty common pattern so create a function for it.

Signed-off-by: Ben Pfaff 
---
 lib/dpif-netdev.c| 6 ++
 lib/jsonrpc.c| 2 +-
 lib/ovsdb-error.c| 6 +++---
 lib/syslog-libc.c| 4 ++--
 lib/util.c   | 8 +++-
 lib/util.h   | 1 +
 lib/vlog.c   | 2 +-
 ofproto/ofproto-dpif-ipfix.c | 6 --
 ofproto/ofproto-dpif-sflow.c | 6 +++---
 ofproto/ofproto.c| 5 ++---
 ovn/utilities/ovn-nbctl.c| 2 +-
 ovn/utilities/ovn-sbctl.c| 2 +-
 ovsdb/replication.c  | 2 +-
 utilities/ovs-vsctl.c| 2 +-
 vswitchd/bridge.c| 8 ++--
 vtep/vtep-ctl.c  | 2 +-
 16 files changed, 29 insertions(+), 35 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 70f320d..ff4227c 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2531,7 +2531,7 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask)
 
 if (!cmask_equals(dp->requested_pmd_cmask, cmask)) {
 free(dp->requested_pmd_cmask);
-dp->requested_pmd_cmask = cmask ? xstrdup(cmask) : NULL;
+dp->requested_pmd_cmask = nullable_xstrdup(cmask);
 }
 
 return 0;
@@ -2690,9 +2690,7 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
 /* Reconfigures the cpu mask. */
 ovs_numa_set_cpu_mask(dp->requested_pmd_cmask);
 free(dp->pmd_cmask);
-dp->pmd_cmask = dp->requested_pmd_cmask
-? xstrdup(dp->requested_pmd_cmask)
-: NULL;
+dp->pmd_cmask = nullable_xstrdup(dp->requested_pmd_cmask);
 
 /* Restores the non-pmd. */
 dp_netdev_set_nonpmd(dp);
diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index 1112b4a..aba742c 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -512,7 +512,7 @@ jsonrpc_create(enum jsonrpc_msg_type type, const char 
*method,
 {
 struct jsonrpc_msg *msg = xmalloc(sizeof *msg);
 msg->type = type;
-msg->method = method ? xstrdup(method) : NULL;
+msg->method = nullable_xstrdup(method);
 msg->params = params;
 msg->result = result;
 msg->error = error;
diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c
index d3549cb..dbe8149 100644
--- a/lib/ovsdb-error.c
+++ b/lib/ovsdb-error.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -183,8 +183,8 @@ ovsdb_error_clone(const struct ovsdb_error *old)
 if (old) {
 struct ovsdb_error *new = xmalloc(sizeof *new);
 new->tag = old->tag;
-new->details = old->details ? xstrdup(old->details) : NULL;
-new->syntax = old->syntax ? xstrdup(old->syntax) : NULL;
+new->details = nullable_xstrdup(old->details);
+new->syntax = nullable_xstrdup(old->syntax);
 new->errno_ = old->errno_;
 return new;
 } else {
diff --git a/lib/syslog-libc.c b/lib/syslog-libc.c
index 8858c3c..b702d41 100644
--- a/lib/syslog-libc.c
+++ b/lib/syslog-libc.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015 Nicira, Inc.
+ * Copyright (c) 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -64,7 +64,7 @@ syslog_libc_open(struct syslogger *this OVS_UNUSED, int 
facility)
  * 'program_name', so make a private copy just for openlog().  (We keep
  * a pointer to the private copy to suppress memory leak warnings in
  * case openlog() does make its own copy.) */
-ident = program_name ? xstrdup(program_name) : NULL;
+ident = nullable_xstrdup(program_name);
 
 openlog(ident, LOG_NDELAY, facility);
 }
diff --git a/lib/util.c b/lib/util.c
index 6ca04ad..e1dc3d2 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, 
Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -151,6 +151,12 @@ xstrdup(const char *s)
 return xmemdup0(s, strlen(s));
 }
 
+char * MALLOC_LIKE
+nullable_xstrdup(const char *s)
+{
+return s ? xstrdup(s) : NULL;
+}
+
 char *
 xvasprintf(const char *format, va_list args)
 {
diff --git a/lib/util.h b/lib/util.h
index 7be4a30..e738c9f 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -112,6 +112,7 @@ void *xrealloc(void *, size_t);
 void *xmemdup(const void *, size_t) MALLOC_LIKE;
 char *xmemdup0(const char *, size_t) MALLOC_LIKE;
 char *xstrdup(const char *) MALLOC_LIKE;
+char *nullable_xstrdup(const char *) MALLOC_LIKE;
 char *xasprintf(const char *format, ...) OVS_PRINTF_FORMAT(1, 2) MALLOC_LIKE;
 char *xvasprintf(const char *format, va_list) OVS_PRINTF_FORMAT(1, 0) 

Re: [ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Ben Pfaff]

On Sat, Jun 25, 2016 at 04:12:04AM +, Daniel Ye wrote:
> It’s because of the "packet send error" statistic. The test of checking IPFIX 
> statistics set collector as “127.0.0.1:4739”.
> Test code:
> -
> dnl Sample every packet using bridge-based sampling.
> AT_CHECK([ovs-vsctl -- set bridge br0 ipfix=@fix -- \
> --id=@fix create ipfix targets=\"127.0.0.1:4739\" \
>   sampling=1], [0], [ignore])
> ——
> 
> We expect “ tx errs=12” because we don’t listen on port 4739 on local host.
> Test code:
> -
> AT_CHECK([ovs-ofctl dump-ipfix-bridge br0], [0], [dnl
> NXST_IPFIX_BRIDGE reply (xid=0x2):
>   bridge ipfix: flows=20, current flows=0, sampled pkts=20, ipv4 ok=0, ipv6 
> ok=0, tx pkts=12
> pkts errs=20, ipv4 errs=20, ipv6 errs=0, tx errs=12
> ])
> ——
> 
> As I talked with Wenyu, she has listened on port 4739 for testing. If port 
> 4739 on the local host is listened, there will
> be no "tx errs”.
> 
> Should we remove "tx errs” check?

OK, that's a bug in the test then.  The tests shouldn't depend on or
require listening on any fixed port numbers.  A lot of tests use the
PARSE_LISTENING_PORT macro to avoid the need to listen on a fixed port.
Here's the definition from ofproto-macros.at:

# PARSE_LISTENING_PORT LOGFILE VARIABLE
#
# Parses the TCP or SSL port on which a server is listening from
# LOGFILE, given that the server was told to listen on a kernel-chosen
# port, and assigns the port number to shell VARIABLE.  You should
# specify the listening remote as ptcp:0:127.0.0.1 or
# pssl:0:127.0.0.1, or the equivalent with [::1] instead of 127.0.0.1.
#
# Here's an example of how to use this with ovsdb-server:
#
#ovsdb-server --log-file --remote=ptcp:0:127.0.0.1 ...
#PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
## Now $TCP_PORT holds the listening port.
m4_define([PARSE_LISTENING_PORT],
[OVS_WAIT_UNTIL([$2=`sed -n 's/.*0:.*: listening on port 
\([[0-9]]*\)$/\1/p' "$1"` && test X != X"[$]$2"])])

start_daemon () {
"$@" -vconsole:off --detach --no-chdir --pidfile --log-file
pid=`cat "$OVS_RUNDIR"/$1.pid`
on_exit "kill $pid"
}

You can find several examples of its use in the tests directory.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Daniel Ye]

Hi Ben,

It’s because of the "packet send error" statistic. The test of checking IPFIX 
statistics set collector as “127.0.0.1:4739”.
Test code:
-
dnl Sample every packet using bridge-based sampling.
AT_CHECK([ovs-vsctl -- set bridge br0 ipfix=@fix -- \
--id=@fix create ipfix targets=\"127.0.0.1:4739\" \
  sampling=1], [0], [ignore])
——

We expect “ tx errs=12” because we don’t listen on port 4739 on local host.
Test code:
-
AT_CHECK([ovs-ofctl dump-ipfix-bridge br0], [0], [dnl
NXST_IPFIX_BRIDGE reply (xid=0x2):
  bridge ipfix: flows=20, current flows=0, sampled pkts=20, ipv4 ok=0, ipv6 
ok=0, tx pkts=12
pkts errs=20, ipv4 errs=20, ipv6 errs=0, tx errs=12
])
——

As I talked with Wenyu, she has listened on port 4739 for testing. If port 4739 
on the local host is listened, there will
be no "tx errs”.

Should we remove "tx errs” check?

Bests,
Daniel

> On Jun 25, 2016, at 11:57 AM, Ben Pfaff  wrote:
> 
> The tests under "make check" try to be environment independent.  Do you
> know what factor of your environment caused the failures?  Perhaps we
> can fix it.
> 
> On Sat, Jun 25, 2016 at 01:11:04AM +, Wenyu Zhang wrote:
>> It is caused by an environment issue.
>> Clear the environment and rerun the tests, all
>> tests passed now.
>> 
>>  iPhone
>> 
>> ? 2016?6?258:35?Wenyu Zhang 
>> > ???
>> 
>> All tests passed except following two cases?
>> 1055: ofproto-dpif - Bridge IPFIX statistics checkFAILED 
>> (ofproto-dpif.at>  >:5975)
>> 1058: ofproto-dpif - Flow IPFIX statistics check  FAILED 
>> (ofproto-dpif.at>  >:6140)
>> 
>> The errors exist without this patch. We are looking at the errors and try to 
>> fix it later.
>> 
>> 
>> ? 2016?6?258:11?Wenyu Zhang 
>> > ???
>> 
>> In virtual network, users want more info about the virtual point to observe 
>> the traffic.
>> It should be a string to provide clear info, not a simple interger ID.
>> 
>> Introduce "other-config: virtual_obs_id" in IPFIX, which is a string 
>> configured by user.
>> Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the 
>> value. The entity is a
>> variable-length string.
>> 
>> Signed-off-by: Wenyu Zhang >
>> ---
>> NEWS  |  11 ++--
>> ofproto/ipfix-enterprise-entities.def |   1 +
>> ofproto/ofproto-dpif-ipfix.c  | 102 
>> ++
>> ofproto/ofproto.h |   2 +
>> vswitchd/bridge.c |  13 +
>> vswitchd/vswitch.xml  |  33 +++
>> 6 files changed, 147 insertions(+), 15 deletions(-)

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

Re: [ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Ben Pfaff]

The tests under "make check" try to be environment independent.  Do you
know what factor of your environment caused the failures?  Perhaps we
can fix it.

On Sat, Jun 25, 2016 at 01:11:04AM +, Wenyu Zhang wrote:
> It is caused by an environment issue.
> Clear the environment and rerun the tests, all
> tests passed now.
> 
>  iPhone
> 
> ? 2016?6?258:35?Wenyu Zhang > 
> ???
> 
> All tests passed except following two cases?
> 1055: ofproto-dpif - Bridge IPFIX statistics checkFAILED 
> (ofproto-dpif.at:5975)
> 1058: ofproto-dpif - Flow IPFIX statistics check  FAILED 
> (ofproto-dpif.at:6140)
> 
> The errors exist without this patch. We are looking at the errors and try to 
> fix it later.
> 
> 
> ? 2016?6?258:11?Wenyu Zhang > 
> ???
> 
> In virtual network, users want more info about the virtual point to observe 
> the traffic.
> It should be a string to provide clear info, not a simple interger ID.
> 
> Introduce "other-config: virtual_obs_id" in IPFIX, which is a string 
> configured by user.
> Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the value. 
> The entity is a
> variable-length string.
> 
> Signed-off-by: Wenyu Zhang >
> ---
> NEWS  |  11 ++--
> ofproto/ipfix-enterprise-entities.def |   1 +
> ofproto/ofproto-dpif-ipfix.c  | 102 ++
> ofproto/ofproto.h |   2 +
> vswitchd/bridge.c |  13 +
> vswitchd/vswitch.xml  |  33 +++
> 6 files changed, 147 insertions(+), 15 deletions(-)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Ben Pfaff]

On Fri, Jun 24, 2016 at 05:10:07PM -0700, Wenyu Zhang wrote:
> In virtual network, users want more info about the virtual point to observe 
> the traffic.
> It should be a string to provide clear info, not a simple interger ID.
> 
> Introduce "other-config: virtual_obs_id" in IPFIX, which is a string 
> configured by user.
> Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the value. 
> The entity is a
> variable-length string.
> 
> Signed-off-by: Wenyu Zhang 

Thanks, I applied this to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports

[Ben Pfaff]

On Fri, Jun 24, 2016 at 08:52:07PM -0700, Ben Pfaff wrote:
> On Thu, Jun 23, 2016 at 01:56:59PM -0400, Hui Kang wrote:
> > 
> > Hi,
> > In our scalability test for OVN, we observed an in-scalable behaviour of
> > the
> > ovn-northd process: the time binding a logical port increases as # of large
> > port increasing, regardless of whether logical ports belong to the same
> > logical
> > switch. The most suspicious function in causing this issue is build_ports()
> > called by ovnnb_db_run() [1], as described below.
> > 
> > Test description:
> > step 1: Create 6 logical switches. For each logical switch, create 200
> > logical ports.
> > step 2: Bind 200 lports from each logical switch on an OVN chassis.
> > 
> > Test results for step 2:
> > 
> > # of ports  |  # of ovn_ports|  Cpu cycle spent in   |
> > | allocated in build_port()  | built_port(), in million  |
> > 200 |200 | 25|
> > 400 |400 | 50|
> > 600 |600 | 75|
> > 800 |800 | 93|
> >1000 |   1000 |108|
> >1200 |   1200 |125|
> 
> I'm surprised that this is expensive for so few ports.  I believe that
> build_ports() runs in O(n) time where n is the larger of the number of
> ports in the northbound and southbound databases.  Does anyone see
> anything that would cause quadratic or more regressive behavior there?

Actually, I take that back.  The cycles/port for all the cases above
demonstrate only slightly nonlinear scaling: 200/25 is 8 Mcycles/port,
1200/125 is 9.6 Mcycles/port.

So the issue is not that it does not scale.  The issue is that it is
slow.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports

[Ben Pfaff]

On Thu, Jun 23, 2016 at 01:56:59PM -0400, Hui Kang wrote:
> 
> Hi,
> In our scalability test for OVN, we observed an in-scalable behaviour of
> the
> ovn-northd process: the time binding a logical port increases as # of large
> port increasing, regardless of whether logical ports belong to the same
> logical
> switch. The most suspicious function in causing this issue is build_ports()
> called by ovnnb_db_run() [1], as described below.
> 
> Test description:
> step 1: Create 6 logical switches. For each logical switch, create 200
> logical ports.
> step 2: Bind 200 lports from each logical switch on an OVN chassis.
> 
> Test results for step 2:
> 
> # of ports  |  # of ovn_ports|  Cpu cycle spent in   |
> | allocated in build_port()  | built_port(), in million  |
> 200 |200 | 25|
> 400 |400 | 50|
> 600 |600 | 75|
> 800 |800 | 93|
>1000 |   1000 |108|
>1200 |   1200 |125|

I'm surprised that this is expensive for so few ports.  I believe that
build_ports() runs in O(n) time where n is the larger of the number of
ports in the northbound and southbound databases.  Does anyone see
anything that would cause quadratic or more regressive behavior there?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports

[Hui Kang]

Ryan Moats/Omaha/IBM wrote on 06/24/2016 10:51:10 PM:

> From: Ryan Moats/Omaha/IBM
> To: Hui Kang/Watson/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 06/24/2016 10:51 PM
> Subject: Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd
> on creating and binding large number of lports
>
> "dev"  wrote on 06/23/2016 12:56:59 PM:
>
> > From: Hui Kang/Watson/IBM@IBMUS
> > To: dev@openvswitch.org
> > Date: 06/23/2016 12:57 PM
> > Subject: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on
> > creating and binding large number of lports
> > Sent by: "dev" 
> >
> >
> > Hi,
> > In our scalability test for OVN, we observed an in-scalable behaviour
of
> > the
> > ovn-northd process: the time binding a logical port increases as # of
large
> > port increasing, regardless of whether logical ports belong to the same
> > logical
> > switch. The most suspicious function in causing this issue is
build_ports()
> > called by ovnnb_db_run() [1], as described below.
> >
> > Test description:
> > step 1: Create 6 logical switches. For each logical switch, create
200
> > logical ports.
> > step 2: Bind 200 lports from each logical switch on an OVN chassis.
> >
> > Test results for step 2:
> >
> > # of ports  |  # of ovn_ports|  Cpu cycle spent in
|
> > | allocated in build_port()  | built_port(), in million
|
> > 200 |200 | 25
|
> > 400 |400 | 50
|
> > 600 |600 | 75
|
> > 800 |800 | 93
|
> >1000 |   1000 |108
|
> >1200 |   1200 |125
|
> >
> > We see that on binding each logical port on a hypervisor,
> > join_logical_ports()
> > in build_port allocates the number of (struct ovn_port) for all the
> > existing
> > ports in the southbound database [2], which causes the accumulated CPU
> > cycles.
> >
> > My question is whether there is any particular reason to allocate that
> > number
> > of (struct ovn_port)? It seems to me there is room in this code to
optimize
> > for performance. Thanks.
> >
> > - Hui
> >
> >
> > [1]
> >
https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2529

> > [2]
> >
https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L571
>
> Hui-
>
> Since it looks like simple short term optimization of northd is a
> "Good Thing" (TM) [1], Is the above the "long pole in the tent" or
> are there other hot spots of similar brightness?
>
> I'm planning at looking at what might be possible, and want to know if
> there are other spots that should be included in the pass.

Hi, Ryan,
Thanks for your attention to this problem. As of now, our profiling data
shows
that the build_ports (including its callee such as join_logical_port) is
the
hot spot.

We can discuss on IRC or slack about how to optimize.

Regards,
- Hui

>
> Thanks in advance,
> Ryan
>
> [1] http://openvswitch.org/pipermail/dev/2016-June/073574.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on creating and binding large number of lports

[Ryan Moats]

"dev"  wrote on 06/23/2016 12:56:59 PM:

> From: Hui Kang/Watson/IBM@IBMUS
> To: dev@openvswitch.org
> Date: 06/23/2016 12:57 PM
> Subject: [ovs-dev] [OVN] Potential scalability bug in ovn-northd on
> creating and binding large number of lports
> Sent by: "dev" 
>
>
> Hi,
> In our scalability test for OVN, we observed an in-scalable behaviour of
> the
> ovn-northd process: the time binding a logical port increases as # of
large
> port increasing, regardless of whether logical ports belong to the same
> logical
> switch. The most suspicious function in causing this issue is build_ports
()
> called by ovnnb_db_run() [1], as described below.
>
> Test description:
> step 1: Create 6 logical switches. For each logical switch, create
200
> logical ports.
> step 2: Bind 200 lports from each logical switch on an OVN chassis.
>
> Test results for step 2:
>
> # of ports  |  # of ovn_ports|  Cpu cycle spent in
|
> | allocated in build_port()  | built_port(), in million
|
> 200 |200 | 25
|
> 400 |400 | 50
|
> 600 |600 | 75
|
> 800 |800 | 93
|
>1000 |   1000 |108
|
>1200 |   1200 |125
|
>
> We see that on binding each logical port on a hypervisor,
> join_logical_ports()
> in build_port allocates the number of (struct ovn_port) for all the
> existing
> ports in the southbound database [2], which causes the accumulated CPU
> cycles.
>
> My question is whether there is any particular reason to allocate that
> number
> of (struct ovn_port)? It seems to me there is room in this code to
optimize
> for performance. Thanks.
>
> - Hui
>
>
> [1]
>
https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2529

> [2]
>
https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L571

Hui-

Since it looks like simple short term optimization of northd is a
"Good Thing" (TM) [1], Is the above the "long pole in the tent" or
are there other hot spots of similar brightness?

I'm planning at looking at what might be possible, and want to know if
there are other spots that should be included in the pass.

Thanks in advance,
Ryan

[1] http://openvswitch.org/pipermail/dev/2016-June/073574.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/4] Restore ovn-controller binding functionality.

[Ryan Moats]

Ben Pfaff  wrote on 06/24/2016 09:03:13 PM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org, Russell Bryant 
> Date: 06/24/2016 09:03 PM
> Subject: Re: [PATCH 0/4] Restore ovn-controller binding functionality.
>
> On Fri, Jun 24, 2016 at 07:57:40PM -0500, Ryan Moats wrote:
> >
> >
> > Ben Pfaff  wrote on 06/24/2016 06:05:16 PM:
> >
> > > From: Ben Pfaff 
> > > To: Ryan Moats/Omaha/IBM@IBMUS
> > > Cc: dev@openvswitch.org, Russell Bryant 
> > > Date: 06/24/2016 06:05 PM
> > > Subject: Re: [PATCH 0/4] Restore ovn-controller binding
functionality.
> > >
> > > On Fri, Jun 24, 2016 at 04:26:24PM -0500, Ryan Moats wrote:
> > > > Ben Pfaff  wrote on 06/24/2016 04:16:14 PM:
> > > > > I applied it before I saw these messages.
> > > > >
> > > > > If it doesn't do the job, and there's no obvious further fix,
it's
> > just
> > > > > one more revert, no big deal.
> > > >
> > > > No worries - it does pass the new test case, which is good and we
are
> > > > running upstream rechecks to see if it does the trick.
> > >
> > > That is very good.
> > >
> > > > BTW, I acked the new test case, with a proviso, so getting that in
> > would be
> > > > nice, in case this doesn't fix things and we need to revert
everything
> > out.
> > >
> > > I agree that we should still add a test case.
> > >
> >
> > Hopefully a committer can take the ack I put on
> > http://patchwork.ozlabs.org/patch/640404/ remove the echo statements
> > and get it pushed in to meet that need.
>
> Russell is a committer so normal practice would be for him to take the
> ack (or request additional reviews if he prefers) and push it himself.

That's useful to know for future reference...

I think the test can wait for the weekend - the upstream jobs looks more
healthy now ...

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

Re: [ovs-dev] [PATCH] debian: Fix upgrade from OVS-2.5 to newer OVS.

[Ben Pfaff]

On Wed, Jun 22, 2016 at 11:30:16AM -0700, Guru Shetty wrote:
> On 17 June 2016 at 12:42, Joe Stringer  wrote:
> 
> > Commit 0dcc739e7a28 ("debian: Move ovs-lib to openvswitch-common.")
> > shifted a file between debian packages, but didn't update the
> > destination package annotations to indicate that it replaces a file
> > from earlier versions of the source package.
> >
> > As a result, if one installs openvswitch-switch-2.5* and then tries to
> > upgrade to openvswitch-{switch,common}-2.5.90+, the install of
> > openvswitch-common will fail like the following:
> >
> > dpkg: error processing archive
> > /tmp/openvswitch-common_2.5.90-1_amd64.deb (--install):
> > trying to overwrite '/usr/share/openvswitch/scripts/ovs-lib', which is
> > also in package openvswitch-switch 2.5.0-1
> >
> > Fix the issue by adding "Replaces" and "Breaks" tags to the new
> > openvswitch-common section of debian/control.
> >
> > Fixes: 0dcc739e7a28 ("debian: Move ovs-lib to openvswitch-common.")
> > Signed-off-by: Joe Stringer 
> >
> 
> One option is to move back ovs-lib to openvswitch-switch and make
> ovn-common depend on openvswitch-switch too. ovn-common is used by
> ovn-central and ovn-host and both of them have a dependency on
> openvswitch-switch anyway, so it is probably not a bad thing.
> 
> Ben do you have any opinions on what is the "debain way" here?

I don't think there's anything wrong with that proposal but one wouldn't
normally make packaging choices on the basis that someone might type
dpkg commands in the wrong order.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] debian: Fix upgrade from OVS-2.5 to newer OVS.

[Ben Pfaff]

dpkg is a pretty low-level tool that users wouldn't ordinarily use.
Users should ordinarily be using "apt" or something else that does
proper sequencing based on dependencies.

On Tue, Jun 21, 2016 at 11:43:53AM -0700, Daniele Di Proietto wrote:
> This looks good to me according to the debian policy manual:
> 
> https://www.debian.org/doc/debian-policy/ch-relationships.html#s-replaces
> 
> Acked-by: Daniele Di Proietto 
> 
> I noticed is that if I try to upgrade from 2.5.0 to 2.5.90(with this
> change) using dpkg from command line the results may vary:
> 
> `dpkg -i openvswitch-common_2.5.90-1_amd64.deb
> openvswitch-switch_2.5.90-1_amd64.deb`
> 
> still complains, because openvswitch-common is processed first and that
> breaks the installed openvswitch-switch. dpkg doesn't seem to be smart
> enough to realize that openvswitch-switch is being updated as well. This is
> the alphabetical order, which is used if someone types `dpkg -i *.deb`
> 
> `dpkg -i openvswitch-switch_2.5.90-1_amd64.deb
> openvswitch-common_2.5.90-1_amd64.deb`
> 
> instead seem to work.
> 
> I'd be curious to hear from someone more expert than me in debian packaging
> if this should be considered a bug in dpkg, or (unlikely, given the
> explicit instructions in the Debian Policy Manual) if there's a better way
> to handle the file transition.
> 
> Thanks,
> 
> Daniele
> 
> 2016-06-17 12:46 GMT-07:00 Joe Stringer :
> 
> > On 17 June 2016 at 12:42, Joe Stringer  wrote:
> > > Commit 0dcc739e7a28 ("debian: Move ovs-lib to openvswitch-common.")
> > > shifted a file between debian packages, but didn't update the
> > > destination package annotations to indicate that it replaces a file
> > > from earlier versions of the source package.
> > >
> > > As a result, if one installs openvswitch-switch-2.5* and then tries to
> > > upgrade to openvswitch-{switch,common}-2.5.90+, the install of
> > > openvswitch-common will fail like the following:
> > >
> > > dpkg: error processing archive
> > > /tmp/openvswitch-common_2.5.90-1_amd64.deb (--install):
> > > trying to overwrite '/usr/share/openvswitch/scripts/ovs-lib', which is
> > > also in package openvswitch-switch 2.5.0-1
> > >
> > > Fix the issue by adding "Replaces" and "Breaks" tags to the new
> > > openvswitch-common section of debian/control.
> >
> > s/new//g
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/4] Restore ovn-controller binding functionality.

[Ben Pfaff]

On Fri, Jun 24, 2016 at 07:57:40PM -0500, Ryan Moats wrote:
> 
> 
> Ben Pfaff  wrote on 06/24/2016 06:05:16 PM:
> 
> > From: Ben Pfaff 
> > To: Ryan Moats/Omaha/IBM@IBMUS
> > Cc: dev@openvswitch.org, Russell Bryant 
> > Date: 06/24/2016 06:05 PM
> > Subject: Re: [PATCH 0/4] Restore ovn-controller binding functionality.
> >
> > On Fri, Jun 24, 2016 at 04:26:24PM -0500, Ryan Moats wrote:
> > > Ben Pfaff  wrote on 06/24/2016 04:16:14 PM:
> > > > I applied it before I saw these messages.
> > > >
> > > > If it doesn't do the job, and there's no obvious further fix, it's
> just
> > > > one more revert, no big deal.
> > >
> > > No worries - it does pass the new test case, which is good and we are
> > > running upstream rechecks to see if it does the trick.
> >
> > That is very good.
> >
> > > BTW, I acked the new test case, with a proviso, so getting that in
> would be
> > > nice, in case this doesn't fix things and we need to revert everything
> out.
> >
> > I agree that we should still add a test case.
> >
> 
> Hopefully a committer can take the ack I put on
> http://patchwork.ozlabs.org/patch/640404/ remove the echo statements
> and get it pushed in to meet that need.

Russell is a committer so normal practice would be for him to take the
ack (or request additional reviews if he prefers) and push it himself.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 17/17] conntrack: Add 'dl_type' parameter to conntrack_execute().

[Daniele Di Proietto]

On 24/06/2016 17:07, "Joe Stringer"  wrote:

>On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
>> Now that dpif_execute has a 'flow' member, it's pretty easy to access a
>> the flow (or the matching megaflow) in dp_execute_cb().
>>
>> This means that's not necessary anymore for the connection tracker to
>> reextract 'dl_type' from the packet, it can be passed as a parameter.
>>
>> This change means that we have to complicate sightly test-conntrack to
>> group the packets by dl_type before passing them to the connection
>> tracker.
>>
>> Signed-off-by: Daniele Di Proietto 
>
>Minor comment below.
>
>Acked-by: Joe Stringer 
>
>> @@ -148,6 +151,44 @@ test_benchmark(struct ovs_cmdl_context *ctx)
>>  }
>>
>>  static void
>> +pcap_batch_execute_conntrack(struct conntrack *ct,
>> + struct dp_packet_batch *pkt_batch)
>> +{
>> +size_t i;
>> +struct dp_packet_batch new_batch;
>> +ovs_be16 dl_type = htons(0);
>> +
>> +dp_packet_batch_init(_batch);
>> +
>> +/* pkt_batch contains packets with different 'dl_type'. We have to
>> + * call conntrack_execute() on packets with the same 'dl_type'. */
>
>Whitespace.

Fixed, thanks for all the reviews!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 15/17] system-tests: Add ping through conntrack test.

[Daniele Di Proietto]

On 24/06/2016 15:41, "Joe Stringer"  wrote:

>On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
>> Signed-off-by: Daniele Di Proietto 
>
>Thanks, I didn't realise this basic test would actually be really
>helpful if there's something fundamentally going wrong in a setup. One
>minor comment below.
>
>Acked-by: Joe Stringer 
>
>> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
>> +
>> +dnl Without this sleep, we get occasional failures due to the following 
>> error:
>> +dnl "connect: Cannot assign requested address"
>> +sleep 2;
>
>Commit c10840ff42da ("system-traffic: Wait for IPv6 connectivity.")
>replaced all of these in the testsuite, could you also update this to
>do the following?
>
>OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2])

Thanks for noticing this! I made the suggested change
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 12/17] flow: Generate checksum in flow_compose().

[Daniele Di Proietto]

On 24/06/2016 15:37, "Joe Stringer"  wrote:

>On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
>> This is useful to test the connection tracker, which performs checksum
>> verification.
>>
>> Signed-off-by: Daniele Di Proietto 
>
>The comment above flow_compose() that says that checksums are missing
>might do with an update.

Good point, thanks!

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

Re: [ovs-dev] [PATCH v4 11/17] flow: Fill udp_len in flow_compose_l4().

[Daniele Di Proietto]

On 24/06/2016 15:31, "Joe Stringer"  wrote:

>On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
>> It will be used by connection tracking tests.
>>
>> Signed-off-by: Daniele Di Proietto 
>
>LGTM. A little surprised this isn't part of another patch, is there
>something particular you want to highlight with this patch?

Ok, I have squashed it with the next patch
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5] ovn: Add software l2 gateway.

[Justin Pettit]

> On Jun 23, 2016, at 12:34 PM, Russell Bryant  wrote:
> 
> This patch implements one approach to using ovn-controller to implement
> a software l2 gateway between logical and physical networks.
> 
> A new logical port type called "l2gateway" is introduced here.  It is very
> close to how localnet ports work, with the following exception:
> 
> - A localnet port makes OVN use the physical network as the
>  transport between hypervisors instead of tunnels. An l2gateway port still
>  uses tunnels between all hypervisors, and packets only go to/from the
>  specified physical network as needed via the chassis the l2gateway port
>  is bound to.
> 
> - An l2gateway port also gets bound to a chassis while a localnet port does
>  not.  This binding is not done by ovn-controller.  It is left as an
>  administrative function.  In the case of OpenStack, the Neutron plugin
>  will do this.

Do you think it's worth mentioning this in the documentation?

> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index a07c327..dcfcd23 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -154,6 +154,15 @@ binding_run(struct controller_ctx *ctx, const struct 
> ovsrec_bridge *br_int,
> }
> sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
> }
> +} else if (!strcmp(binding_rec->type, "l2gateway")
> +   && binding_rec->chassis == chassis_rec) {
> +/* A locally bound gateway port.
> + *
> + * ovn-controller does not bind gateway ports itself.
> + * Choosing a chassis for a gateway port is left
> + * up to an entity external to OVN. */

I'd specify "L2 gateway in the comments above, since right below there's the L3 
gateway.

> +sset_add(all_lports, binding_rec->logical_port);
> +add_local_datapath(local_datapaths, binding_rec);
> } else if (chassis_rec && binding_rec->chassis == chassis_rec
>&& strcmp(binding_rec->type, "gateway")) {
> if (ctx->ovnsb_idl_txn) {

...

> +  
> +
> +  These options apply when  is
> +  l2gateway.
> +
> +
> +
> +  Required.  The name of the network to which the 
> l2gateway
> +  port is connected.  The gateway, via ovn-controller,
> +  uses its local configuration to determine exactly how to connect to
> +  this network.
> +
> +  

It looks like L3 gateways are bound by setting the "chassis" option in logical 
routers, but L2 gateways are bound by setting "ovn-bridge-mappings" in the 
Southbound database.  It's a bit unfortunate that we're using pretty different 
methods for similar actions.  Do you think there's a way that we can make them 
more similar?

I know it's a hot-button issue, but I wonder if it's worth dusting off the 
logical-physical separation conversation again.

> diff --git a/ovn/controller/ovn-controller.8.xml 
> b/ovn/controller/ovn-controller.8.xml
> index 1ee3a6e..228a8cd 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> ...
> @@ -199,6 +199,29 @@
>   
> 
>   
> +external-ids:ovn-gateway-port in the Port
> +table
> +  
> +  
> +
> +  The presence of this key identifies a patch port as one created by
> +  ovn-controller to connect the integration bridge and
> +  another bridge to implement a gateway logical port.
> +  Its value is the name of the logical port with type
> +  set to gateway that the port implements. See
> +  external_ids:ovn-bridge-mappings, above, for more
> +  information.
> +
> +
> +
> +  Each gateway logical port is implemented as a pair
> +  of patch ports, one in the integration bridge, one in a different
> +  bridge, with the same external-ids:ovn-gateway-port
> +  value.
> +

Should these "gateway" references be renamed "l2gateway"?

> diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
> index 652466b..edf3baf 100644
> --- a/ovn/controller/patch.c
> +++ b/ovn/controller/patch.c
> ...
> @@ -195,31 +197,40 @@ add_bridge_mappings(struct controller_ctx *ctx,
> continue;
> }
> ld->localnet_port = binding;
> +patch_port_id = "ovn-localnet-port";
> +} else if (!strcmp(binding->type, "l2gateway")) {
> +if (!binding->chassis || strcmp(chassis_id, 
> binding->chassis->name)) {
> +/* This gateway port is not bound to this chassis, so we 
> should
> + * not create any patch ports for it. */
> +continue;
> +}

The usual comments about referring to it as "L2 gateway".  I'll be pointing out 
a few more instances of this, but you might do a general pass to clarify spots 
that just refer to "gateway".

[ovs-dev] [PATCH] datapath-windows: Conntrack - Fix variable initialization

[Sairam Venugopal]

Initialize the variable pktMdLabel.

Signed-off-by: Sairam Venugopal 
---
 datapath-windows/ovsext/Conntrack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Conntrack.c 
b/datapath-windows/ovsext/Conntrack.c
index ebfdeef..bb28b65 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -500,7 +500,7 @@ OvsConntrackSetLabels(OvsFlowKey *key,
   struct ovs_key_ct_labels *val,
   struct ovs_key_ct_labels *mask)
 {
-ovs_u128 v, m, pktMdLabel;
+ovs_u128 v, m, pktMdLabel = {0};
 memcpy(, val, sizeof v);
 memcpy(, mask, sizeof m);
 
-- 
2.5.0.windows.1

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

[ovs-dev] [PATCH] datapath-windows: Cleanup conntrack-tcp.c

[Sairam Venugopal]

Update the code to use tcp->flags. This keeps the kernel conntrack-tcp.c file 
in sync with userspace version.

This patch also addresses an warning - 'Comparison of a boolean expression with 
an integer other than 0 or 1' - (tcp_flags & (TCP_ACK|TCP_RST)) == 
(TCP_ACK|TCP_RST))

Signed-off-by: Sairam Venugopal 
---
 datapath-windows/ovsext/Conntrack-tcp.c | 79 +++--
 datapath-windows/ovsext/NetProto.h  | 25 ++-
 2 files changed, 51 insertions(+), 53 deletions(-)

diff --git a/datapath-windows/ovsext/Conntrack-tcp.c 
b/datapath-windows/ovsext/Conntrack-tcp.c
index a0ee791..1820705 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -128,27 +128,22 @@ enum ct_dpif_tcp_state {
 /* pf does this in in pf_normalize_tcp(), and it is called only if scrub
  * is enabled.  We're not scrubbing, but this check seems reasonable.  */
 static __inline BOOLEAN
-OvsConntrackValidateTcpFlags(const TCPHdr *tcp)
+OvsCtInvalidTcpFlags(uint16_t flags)
 {
-if (tcp->syn) {
-if (tcp->rst) {
-return TRUE;
-}
-if (tcp->fin) {
-/* Here pf removes the fin flag.  We simply mark the packet as
- * invalid */
+if (flags & TCP_SYN) {
+if (flags & TCP_RST || flags & TCP_FIN) {
 return TRUE;
 }
 } else {
 /* Illegal packet */
-if (!(tcp->ack || tcp->rst)) {
+if (!(flags & (TCP_ACK|TCP_RST))) {
 return TRUE;
 }
 }
 
-if (!(tcp->ack)) {
+if (!(flags & TCP_ACK)) {
 /* These flags are only valid if ACK is set */
-if ((tcp->fin) || (tcp->psh) || (tcp->urg)) {
+if ((flags & TCP_FIN) || (flags & TCP_PSH) || (flags & TCP_URG)) {
 return TRUE;
 }
 }
@@ -165,10 +160,9 @@ OvsTcpGetWscale(const TCPHdr *tcp)
 uint8_t optlen;
 
 while (len >= 3) {
-if (*opt == TCPOPT_EOL) {
-break;
-}
 switch (*opt) {
+case TCPOPT_EOL:
+return wscale;
 case TCPOPT_NOP:
 opt++;
 len--;
@@ -232,31 +226,33 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
 /* The peer that should receive 'pkt' */
 struct tcp_peer *dst = >peer[reply ? 0 : 1];
 uint8_t sws = 0, dws = 0;
+UINT16 tcp_flags = tcp->flags;
 uint16_t win = ntohs(tcp->window);
 uint32_t ack, end, seq, orig_seq;
 uint32_t p_len = OvsGetTcpPayloadLength(nbl);
 int ackskew;
 
-if (OvsConntrackValidateTcpFlags(tcp)) {
+if (OvsCtInvalidTcpFlags(tcp->flags)) {
 return CT_UPDATE_INVALID;
 }
 
-if ((tcp->syn) && dst->state >= CT_DPIF_TCPS_FIN_WAIT_2 &&
-src->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
+if (((tcp_flags & (TCP_SYN|TCP_ACK)) == TCP_SYN)
+&& dst->state >= CT_DPIF_TCPS_FIN_WAIT_2
+&& src->state >= CT_DPIF_TCPS_FIN_WAIT_2) {
 src->state = dst->state = CT_DPIF_TCPS_CLOSED;
 return CT_UPDATE_NEW;
 }
 
 if (src->wscale & CT_WSCALE_FLAG
 && dst->wscale & CT_WSCALE_FLAG
-&& !(tcp->syn)) {
+&& !(tcp_flags & TCP_SYN)) {
 
 sws = src->wscale & CT_WSCALE_MASK;
 dws = dst->wscale & CT_WSCALE_MASK;
 
 } else if (src->wscale & CT_WSCALE_UNKNOWN
 && dst->wscale & CT_WSCALE_UNKNOWN
-&& !(tcp->syn)) {
+&& !(tcp_flags & TCP_SYN)) {
 
 sws = TCP_MAX_WSCALE;
 dws = TCP_MAX_WSCALE;
@@ -275,7 +271,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
 ack = ntohl(tcp->ack);
 
 end = seq + p_len;
-if (tcp->syn) {
+if (tcp_flags & TCP_SYN) {
 end++;
 if (dst->wscale & CT_WSCALE_FLAG) {
 src->wscale = OvsTcpGetWscale(tcp);
@@ -286,14 +282,13 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
 dws = dst->wscale & CT_WSCALE_MASK;
 } else {
 /* fixup other window */
-dst->max_win <<= dst->wscale &
-CT_WSCALE_MASK;
+dst->max_win <<= dst->wscale & CT_WSCALE_MASK;
 /* in case of a retrans SYN|ACK */
 dst->wscale = 0;
 }
 }
 }
-if (tcp->fin) {
+if (tcp_flags & TCP_FIN) {
 end++;
 }
 
@@ -305,8 +300,7 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
  * after establishment)
  */
 if (src->seqhi == 1 ||
-SEQ_GEQ(end + MAX(1, dst->max_win << dws),
-src->seqhi)) {
+SEQ_GEQ(end + MAX(1, dst->max_win << dws), src->seqhi)) {
 src->seqhi = end + MAX(1, dst->max_win << dws);
 }
 if (win > src->max_win) {
@@ -316,19 +310,19 @@ OvsConntrackUpdateTcpEntry(OVS_CT_ENTRY* conn_,
 } else {
 ack = ntohl(tcp->ack);
 end = seq + p_len;
-

Re: [ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Wenyu Zhang]

It is caused by an environment issue.
Clear the environment and rerun the tests, all
tests passed now.

 iPhone

? 2016?6?258:35?Wenyu Zhang > 
???

All tests passed except following two cases?
1055: ofproto-dpif - Bridge IPFIX statistics checkFAILED 
(ofproto-dpif.at:5975)
1058: ofproto-dpif - Flow IPFIX statistics check  FAILED 
(ofproto-dpif.at:6140)

The errors exist without this patch. We are looking at the errors and try to 
fix it later.


? 2016?6?258:11?Wenyu Zhang > 
???

In virtual network, users want more info about the virtual point to observe the 
traffic.
It should be a string to provide clear info, not a simple interger ID.

Introduce "other-config: virtual_obs_id" in IPFIX, which is a string configured 
by user.
Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the value. 
The entity is a
variable-length string.

Signed-off-by: Wenyu Zhang >
---
NEWS  |  11 ++--
ofproto/ipfix-enterprise-entities.def |   1 +
ofproto/ofproto-dpif-ipfix.c  | 102 ++
ofproto/ofproto.h |   2 +
vswitchd/bridge.c |  13 +
vswitchd/vswitch.xml  |  33 +++
6 files changed, 147 insertions(+), 15 deletions(-)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/4] Restore ovn-controller binding functionality.

[Ryan Moats]

Ben Pfaff  wrote on 06/24/2016 06:05:16 PM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org, Russell Bryant 
> Date: 06/24/2016 06:05 PM
> Subject: Re: [PATCH 0/4] Restore ovn-controller binding functionality.
>
> On Fri, Jun 24, 2016 at 04:26:24PM -0500, Ryan Moats wrote:
> > Ben Pfaff  wrote on 06/24/2016 04:16:14 PM:
> > > I applied it before I saw these messages.
> > >
> > > If it doesn't do the job, and there's no obvious further fix, it's
just
> > > one more revert, no big deal.
> >
> > No worries - it does pass the new test case, which is good and we are
> > running upstream rechecks to see if it does the trick.
>
> That is very good.
>
> > BTW, I acked the new test case, with a proviso, so getting that in
would be
> > nice, in case this doesn't fix things and we need to revert everything
out.
>
> I agree that we should still add a test case.
>

Hopefully a committer can take the ack I put on
http://patchwork.ozlabs.org/patch/640404/ remove the echo statements
and get it pushed in to meet that need.

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

Re: [ovs-dev] [PATCH V2] ipfix: Export user specified virtual observation ID

[Wenyu Zhang]

I have fixed the issue, and sent out a new patch. 

Due to the result of this feature only can been seen in the exported IPFIX 
packet, the test case for it may be a big work. We may consider it with other 
IPFIX features tests a litter later.





发自我的 iPhone
> 在 2016年6月25日，上午6:20，Wenyu Zhang  写道：
> 
> Thanks for your catch.
> Iam working on it.
> 
> 发自我的 iPhone
> 
>> 在 2016年6月25日，上午4:55，William Tu  写道：
>> 
>> Hi Wenyu,
>> 
>> I was debugging a little bit and the issue is a NULL pointer deference
>> of be_cfg at   virtual_obs_id = smap_get(_cfg->other_config,
>> "virtual_obs_id");
>> 
>> Maybe adding if (valid_be_cfg) check before the deference? I will
>> leave you to fix it. Also I hope you can add a test case to this case.
>> 
>> Regards,
>> William
>> 
>> 
 On Fri, Jun 24, 2016 at 1:42 PM, Ben Pfaff  wrote:
 On Fri, Jun 24, 2016 at 05:25:57AM -0700, Wenyu Zhang wrote:
 In virtual network, users want more info about the virtual point to 
 observe the traffic.
 It should be a string to provide clear info, not a simple interger ID.
 
 Introduce "other-config: virtual_obs_id" in IPFIX, which is a string 
 configured by user.
 Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the 
 value. The entity is a
 variable-length string.
 
 Signed-off-by: Wenyu Zhang 
>>> 
>>> Hi Wenyu.
>>> 
>>> I reverted this commit because it dumps core in test 1048 "ofproto-dpif
>>> - Flow IPFIX sanity check" with the following backtrace.  It crashes
>>> the same way whether I use your original commit or my modified one.
>>> 
>>> #0  hmap_first_with_hash (hmap=, hmap=,
>>>   hash=) at ../lib/hmap.h:328
>>> #1  smap_find__ (smap=0x94, key=key@entry=0x817f7ab "virtual_obs_id",
>>>   key_len=14, hash=2537071222) at ../lib/smap.c:366
>>> #2  0x0812b9d7 in smap_get_node (smap=0x9738a276,
>>>   key=0x817f7ab "virtual_obs_id") at ../lib/smap.c:198
>>> #3  0x0812ba30 in smap_get (smap=0x94, key=0x817f7ab "virtual_obs_id")
>>>   at ../lib/smap.c:189
>>> #4  0x08055a60 in bridge_configure_ipfix (br=)
>>>   at ../vswitchd/bridge.c:1237
>>> #5  bridge_reconfigure (ovs_cfg=0x94) at ../vswitchd/bridge.c:666
>>> #6  0x080568d3 in bridge_run () at ../vswitchd/bridge.c:2972
>>> #7  0x0804c9dd in main (argc=10, argv=0xffd8b934)
>>>   at ../vswitchd/ovs-vswitchd.c:112
>>> 
>>> When you're ready, please post a fixed version.  Please start from the
>>> version that I committed.
>>> 
>>> Before you post a patch, run the unit tests.
>>> ___
>>> dev mailing list
>>> dev@openvswitch.org
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev=CwIBaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=MJBX2NeoFFE5o-wzhi1dy7zWKBNufpc6ky5q7EaQJBo=YK-ciEyP0lB9loxNsFHuW5TCW0xNKneVS3R9D-CunQo=GYVyJZTPKYoaidcS5nQES3WHzwOjvINyEuv41aVqlTg=
>>>  
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Wenyu Zhang]

All tests passed except following two cases?
1055: ofproto-dpif - Bridge IPFIX statistics checkFAILED 
(ofproto-dpif.at:5975)
1058: ofproto-dpif - Flow IPFIX statistics check  FAILED 
(ofproto-dpif.at:6140)

The errors exist without this patch. We are looking at the errors and try to 
fix it later.


? 2016?6?258:11?Wenyu Zhang > 
???

In virtual network, users want more info about the virtual point to observe the 
traffic.
It should be a string to provide clear info, not a simple interger ID.

Introduce "other-config: virtual_obs_id" in IPFIX, which is a string configured 
by user.
Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the value. 
The entity is a
variable-length string.

Signed-off-by: Wenyu Zhang >
---
NEWS  |  11 ++--
ofproto/ipfix-enterprise-entities.def |   1 +
ofproto/ofproto-dpif-ipfix.c  | 102 ++
ofproto/ofproto.h |   2 +
vswitchd/bridge.c |  13 +
vswitchd/vswitch.xml  |  33 +++
6 files changed, 147 insertions(+), 15 deletions(-)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 05/17] conntrack: Periodically delete expired connections.

[Joe Stringer]

On 24 June 2016 at 16:42, Daniele Di Proietto  wrote:
> Thanks for your comments Joe, replies inline
>
>
>
> On 24/06/2016 15:29, "Joe Stringer"  wrote:
>
>>On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
>>> This commit adds a thread that periodically removes expired connections.
>>>
>>> The expiration time of a connection can be expressed by:
>>>
>>> expiration = now + timeout
>>>
>>> For each possible 'timeout' value (there aren't many) we keep a list.
>>> When the expiration is updated, we move the connection to the back of the
>>> corresponding 'timeout' list. This ways, the list is always ordered by
>>> 'expiration'.
>>>
>>> When the cleanup thread iterates through the lists for expired
>>> connections, it can stop at the first non expired connection.
>>>
>>> Suggested-by: Joe Stringer 
>>> Signed-off-by: Daniele Di Proietto 
>>> ---
>>>  lib/conntrack-other.c   |  11 +--
>>>  lib/conntrack-private.h |  21 --
>>>  lib/conntrack-tcp.c |  20 +++---
>>>  lib/conntrack.c | 184 
>>> 
>>>  lib/conntrack.h |  32 -
>>>  5 files changed, 237 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/lib/conntrack-other.c b/lib/conntrack-other.c
>>> index 295cb2c..2920889 100644
>>> --- a/lib/conntrack-other.c
>>> +++ b/lib/conntrack-other.c
>>> @@ -43,8 +43,8 @@ conn_other_cast(const struct conn *conn)
>>>  }
>>>
>>>  static enum ct_update_res
>>> -other_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
>>> -  bool reply, long long now)
>>> +other_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
>>> +  struct dp_packet *pkt OVS_UNUSED, bool reply, long long 
>>> now)
>>>  {
>>>  struct conn_other *conn = conn_other_cast(conn_);
>>>
>>> @@ -54,7 +54,7 @@ other_conn_update(struct conn *conn_, struct dp_packet 
>>> *pkt OVS_UNUSED,
>>>  conn->state = OTHERS_MULTIPLE;
>>>  }
>>>
>>> -update_expiration(conn_, other_timeouts[conn->state], now);
>>> +conn_update_expiration(ctb, >up, othler_timeouts[conn->state], 
>>> now);
>>>
>>>  return CT_UPDATE_VALID;
>>>  }
>>> @@ -66,14 +66,15 @@ other_valid_new(struct dp_packet *pkt OVS_UNUSED)
>>>  }
>>>
>>>  static struct conn *
>>> -other_new_conn(struct dp_packet *pkt OVS_UNUSED, long long now)
>>> +other_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt 
>>> OVS_UNUSED,
>>> +   long long now)
>>>  {
>>>  struct conn_other *conn;
>>>
>>>  conn = xzalloc(sizeof *conn);
>>>  conn->state = OTHERS_FIRST;
>>>
>>> -update_expiration(>up, other_timeouts[conn->state], now);
>>> +conn_init_expiration(ctb, >up, other_timeouts[conn->state], now);
>>>
>>>  return >up;
>>>  }
>>> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
>>> index d3e0099..4743dc6 100644
>>> --- a/lib/conntrack-private.h
>>> +++ b/lib/conntrack-private.h
>>> @@ -68,10 +68,13 @@ enum ct_update_res {
>>>  };
>>>
>>>  struct ct_l4_proto {
>>> -struct conn *(*new_conn)(struct dp_packet *pkt, long long now);
>>> +struct conn *(*new_conn)(struct conntrack_bucket *, struct dp_packet 
>>> *pkt,
>>> + long long now);
>>>  bool (*valid_new)(struct dp_packet *pkt);
>>> -enum ct_update_res (*conn_update)(struct conn *conn, struct dp_packet 
>>> *pkt,
>>> -  bool reply, long long now);
>>> +enum ct_update_res (*conn_update)(struct conn *conn,
>>> +  struct conntrack_bucket *,
>>> +  struct dp_packet *pkt, bool reply,
>>> +  long long now);
>>>  };
>>>
>>>  extern struct ct_l4_proto ct_proto_tcp;
>>> @@ -80,9 +83,19 @@ extern struct ct_l4_proto ct_proto_other;
>>>  extern long long ct_timeout_val[];
>>>
>>>  static inline void
>>> -update_expiration(struct conn *conn, enum ct_timeout tm, long long now)
>>> +conn_init_expiration(struct conntrack_bucket *ctb, struct conn *conn,
>>> +enum ct_timeout tm, long long now)
>>>  {
>>>  conn->expiration = now + ct_timeout_val[tm];
>>> +ovs_list_push_back(>exp_lists[tm], >exp_node);
>>> +}
>>> +
>>> +static inline void
>>> +conn_update_expiration(struct conntrack_bucket *ctb, struct conn *conn,
>>> +   enum ct_timeout tm, long long now)
>>> +{
>>> +ovs_list_remove(>exp_node);
>>> +conn_init_expiration(ctb, conn, tm, now);
>>>  }
>>>
>>>  #endif /* conntrack-private.h */
>>> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
>>> index b574eeb..71eadc1 100644
>>> --- a/lib/conntrack-tcp.c
>>> +++ b/lib/conntrack-tcp.c
>>> @@ -152,8 +152,8 @@ tcp_payload_length(struct dp_packet *pkt)
>>>  }
>>>
>>>  static enum ct_update_res
>>> -tcp_conn_update(struct conn* conn_, struct dp_packet *pkt, bool reply,
>>> -  

Re: [ovs-dev] [PATCH 4/4] ovsdb: Add unixctl commands for OVSDB replication

[Ben Pfaff]

On Fri, Jun 24, 2016 at 04:44:05PM -0700, Ben Pfaff wrote:
> From: Mario Cabrera 
> 
> Set and get the server to replicate from:
> 
>   ovsdb-server/set-remote-ovsdb-server {server}
>   ovsdb-server/get-remote-ovsdb-server
> 
> Set and get the replicated table blacklist:
> 
>   ovsdb-server/set-sync-excluded-tables {DB:table,...}
>   ovsdb-server/get-sync-excluded-tables
> 
> Connect to the configured server and start replication:
> 
>   ovsdb-server/connect-remote-ovsdb-server
> 
> Disconnect from the remote server and stop replication, without dropping
> the replicated data:
> 
>   ovsdb-server/disconnect-remote-ovsdb-server
> 
> Signed-off-by: Mario Cabrera 

When I apply this, the new unit test fails.  Does it pass for you?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] sset: New function sset_from_delimited_string().

[Ben Pfaff]

This simplifies code in a couple of places.

Signed-off-by: Ben Pfaff 
---
 lib/sset.c| 21 -
 lib/sset.h|  3 +++
 ovn/utilities/ovn-sbctl.c | 10 ++
 ovsdb/replication.c   | 12 +---
 4 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/lib/sset.c b/lib/sset.c
index 4fd3fae..2913097 100644
--- a/lib/sset.c
+++ b/lib/sset.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2011, 2012, 2013, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -99,6 +99,25 @@ sset_moved(struct sset *set)
 hmap_moved(>map);
 }
 
+/* Initializes 'set' with substrings of 's' that are delimited by any of the
+ * characters in 'delimiters'.  For example,
+ * sset_from_delimited_string(, "a b,c", " ,");
+ * initializes 'set' with three strings "a", "b", and "c". */
+void
+sset_from_delimited_string(struct sset *set, const char *s_,
+   const char *delimiters)
+{
+sset_init(set);
+
+char *s = xstrdup(s_);
+char *token, *save_ptr = NULL;
+for (token = strtok_r(s, delimiters, _ptr); token != NULL;
+ token = strtok_r(NULL, delimiters, _ptr)) {
+sset_add(set, token);
+}
+free(tokstr);
+}
+
 /* Returns true if 'set' contains no strings, false if it contains at least one
  * string. */
 bool
diff --git a/lib/sset.h b/lib/sset.h
index 9c2f703..c3b5e97 100644
--- a/lib/sset.h
+++ b/lib/sset.h
@@ -43,6 +43,9 @@ void sset_clone(struct sset *, const struct sset *);
 void sset_swap(struct sset *, struct sset *);
 void sset_moved(struct sset *);
 
+void sset_from_delimited_string(struct sset *, const char *s,
+const char *delimiters);
+
 /* Count. */
 bool sset_is_empty(const struct sset *);
 size_t sset_count(const struct sset *);
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index c0ee518..61d9cf5 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -548,14 +548,8 @@ cmd_chassis_add(struct ctl_context *ctx)
 check_conflicts(sbctl_ctx, ch_name,
 xasprintf("cannot create a chassis named %s", ch_name));
 
-char *tokstr = xstrdup(encap_types);
-char *token, *save_ptr = NULL;
-struct sset encap_set = SSET_INITIALIZER(_set);
-for (token = strtok_r(tokstr, ",", _ptr); token != NULL;
- token = strtok_r(NULL, ",", _ptr)) {
-sset_add(_set, token);
-}
-free(tokstr);
+struct sset encap_set;
+sset_from_delimited_string(encap_types, _set, ",");
 
 size_t n_encaps = sset_count(_set);
 struct sbrec_encap **encaps = xmalloc(n_encaps * sizeof *encaps);
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 1fa0f25..aa6b9e2 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -121,19 +121,9 @@ set_remote_ovsdb_server(const char *remote_server)
 void
 set_tables_blacklist(const char *blacklist)
 {
-char *save_ptr = NULL;
-char *blacklist_item;
-
 replication_init();
-
 if (blacklist) {
-char *t_blacklist = xstrdup(blacklist);
-for (blacklist_item = strtok_r(t_blacklist, ",", _ptr);
- blacklist_item != NULL;
- blacklist_item = strtok_r(NULL, ",", _ptr)) {
-sset_add(_blacklist, blacklist_item);
-}
-free(t_blacklist);
+sset_from_delimited_string(_blacklist, blacklist, ",");
 }
 }
 
-- 
2.1.3

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

[ovs-dev] [PATCH V3] ipfix: Export user specified virtual observation ID

[Wenyu Zhang]

In virtual network, users want more info about the virtual point to observe the 
traffic.
It should be a string to provide clear info, not a simple interger ID.

Introduce "other-config: virtual_obs_id" in IPFIX, which is a string configured 
by user.
Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the value. 
The entity is a
variable-length string.

Signed-off-by: Wenyu Zhang 
---
 NEWS  |  11 ++--
 ofproto/ipfix-enterprise-entities.def |   1 +
 ofproto/ofproto-dpif-ipfix.c  | 102 ++
 ofproto/ofproto.h |   2 +
 vswitchd/bridge.c |  13 +
 vswitchd/vswitch.xml  |  33 +++
 6 files changed, 147 insertions(+), 15 deletions(-)

diff --git a/NEWS b/NEWS
index d00b183..7aa050b 100644
--- a/NEWS
+++ b/NEWS
@@ -15,16 +15,19 @@ Post-v2.5.0
now implemented.  Only flow mod and port mod messages are supported
in bundles.
  * New OpenFlow extension NXM_NX_MPLS_TTL to provide access to MPLS TTL.
- * New "sampling_port" option for "sample" action to allow sampling
-   ingress and egress tunnel metadata with IPFIX.
  * New output option, output(port=N,max_len=M), to allow truncating a
packet to size M bytes when outputting to port N.
- ovs-ofctl:
  * queue-get-config command now allows a queue ID to be specified.
  * '--bundle' option can now be used with OpenFlow 1.3.
  * New option "--color" to produce colorized output for some commands.
- * New commands "dump-ipfix-bridge" and "dump-ipfix-flow" to dump bridge
-   IPFIX statistics and flow based IPFIX statistics.
+   - IPFIX:
+ * New "sampling_port" option for "sample" action to allow sampling
+   ingress and egress tunnel metadata with IPFIX.
+ * New ovs-ofctl commands "dump-ipfix-bridge" and "dump-ipfix-flow" to
+   dump bridge IPFIX statistics and flow based IPFIX statistics.
+ * New setting other-config:virtual_obs_id to add an arbitrary string
+   to IPFIX records.
- Linux:
  * New QoS type "linux-noop" that prevents Open vSwitch from trying to
manage QoS for a given port (useful when other software manages QoS).
diff --git a/ofproto/ipfix-enterprise-entities.def 
b/ofproto/ipfix-enterprise-entities.def
index ea94586..73a520c 100644
--- a/ofproto/ipfix-enterprise-entities.def
+++ b/ofproto/ipfix-enterprise-entities.def
@@ -12,5 +12,6 @@ IPFIX_ENTERPRISE_ENTITY(TUNNEL_DESTINATION_IPV4_ADDRESS, 894, 
4, tunnelDestinati
 IPFIX_ENTERPRISE_ENTITY(TUNNEL_PROTOCOL_IDENTIFIER, 895, 1, 
tunnelProtocolIdentifier, IPFIX_ENTERPRISE_VMWARE)
 IPFIX_ENTERPRISE_ENTITY(TUNNEL_SOURCE_TRANSPORT_PORT, 896, 2, 
tunnelSourceTransportPort, IPFIX_ENTERPRISE_VMWARE)
 IPFIX_ENTERPRISE_ENTITY(TUNNEL_DESTINATION_TRANSPORT_PORT, 897, 2, 
tunnelDestinationTransportPort, IPFIX_ENTERPRISE_VMWARE)
+IPFIX_ENTERPRISE_ENTITY(VIRTUAL_OBS_ID, 898, 0, virtualObsID, 
IPFIX_ENTERPRISE_VMWARE)
 
 #undef IPFIX_ENTERPRISE_ENTITY
diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index f166014..35f481d 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -101,6 +101,8 @@ struct dpif_ipfix_exporter {
 struct ovs_list cache_flow_start_timestamp_list;  /* 
ipfix_flow_cache_entry. */
 uint32_t cache_active_timeout;  /* In seconds. */
 uint32_t cache_max_flows;
+char *virtual_obs_id;
+uint8_t virtual_obs_len;
 
 ofproto_ipfix_stats stats;
 };
@@ -367,6 +369,32 @@ struct ipfix_data_record_aggregated_ip {
 BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_ip) == 32);
 
 /*
+ * Refer to RFC 7011, the length of Variable length element is 0~65535:
+ * In most case, it should be less than 255 octets:
+ *  0   1   2   3
+ *  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ *  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *  | Length (< 255)|  Information Element  |
+ *  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *  |  ... continuing as needed |
+ *  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * When it is greater than or equeal to 255 octets:
+ *  0   1   2   3
+ *  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ *  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *  |  255  |  Length (0 to 65535)  |   IE  |
+ *  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *  |  ... continuing as needed |
+ *  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ *
+ * Now, only the virtual_obs_id whose length < 255 is implemented.
+ */
+
+#define IPFIX_VIRTUAL_OBS_MAX_LEN 254
+
+/*
  * support 

Re: [ovs-dev] [PATCH v4 17/17] conntrack: Add 'dl_type' parameter to conntrack_execute().

[Joe Stringer]

On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
> Now that dpif_execute has a 'flow' member, it's pretty easy to access a
> the flow (or the matching megaflow) in dp_execute_cb().
>
> This means that's not necessary anymore for the connection tracker to
> reextract 'dl_type' from the packet, it can be passed as a parameter.
>
> This change means that we have to complicate sightly test-conntrack to
> group the packets by dl_type before passing them to the connection
> tracker.
>
> Signed-off-by: Daniele Di Proietto 

Minor comment below.

Acked-by: Joe Stringer 

> @@ -148,6 +151,44 @@ test_benchmark(struct ovs_cmdl_context *ctx)
>  }
>
>  static void
> +pcap_batch_execute_conntrack(struct conntrack *ct,
> + struct dp_packet_batch *pkt_batch)
> +{
> +size_t i;
> +struct dp_packet_batch new_batch;
> +ovs_be16 dl_type = htons(0);
> +
> +dp_packet_batch_init(_batch);
> +
> +/* pkt_batch contains packets with different 'dl_type'. We have to
> + * call conntrack_execute() on packets with the same 'dl_type'. */

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

Re: [ovs-dev] [PATCH 3/4] ovsdb: Add table exclusion functionality to OVSDB replication

[Ben Pfaff]

On Fri, Jun 24, 2016 at 04:44:04PM -0700, Ben Pfaff wrote:
> From: Mario Cabrera 
> 
> A blacklist of tables that will be excluded from replication can be
> specified by the following option:
> 
> --sync-exclude-tables=db:table[,db:table]…
> 
> Where 'table' corresponds to a table name, and 'db' corresponds to the
> database name where the table resides.
> 
> Signed-off-by: Mario Cabrera 

Thanks for the patch.

I noticed some indentations to fix.  Also, there is some code that tries
too hard to allocate a string with exactly the right length; I replaced
it with a call to xasprintf().

I folded in the following and I plan to apply patches 1 to 3 to master
in a minute.

--8<--cut here-->8--

diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index 8cea620..1fa0f25 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -127,7 +127,7 @@ set_tables_blacklist(const char *blacklist)
 replication_init();
 
 if (blacklist) {
-   char *t_blacklist = strdup(blacklist);
+char *t_blacklist = xstrdup(blacklist);
 for (blacklist_item = strtok_r(t_blacklist, ",", _ptr);
  blacklist_item != NULL;
  blacklist_item = strtok_r(NULL, ",", _ptr)) {
@@ -190,14 +190,9 @@ reset_database(struct ovsdb *db, struct ovsdb_txn *txn)
 struct ovsdb_table *table = table_node->data;
 struct ovsdb_row *row;
 
-size_t blacklist_item_len = strlen(db->schema->name) +
-strlen(table_node->name) + 2;
-
 /* Do not reset if table is blacklisted. */
-char* blacklist_item = xmalloc(blacklist_item_len);
-snprintf(blacklist_item, blacklist_item_len, "%s%s%s",
- db->schema->name, ":", table_node->name);
-
+char *blacklist_item = xasprintf(
+"%s%s%s", db->schema->name, ":", table_node->name);
 if (!sset_contains(_blacklist, blacklist_item)) {
 HMAP_FOR_EACH (row, hmap_node, >rows) {
 ovsdb_txn_row_delete(txn, row);
@@ -334,14 +329,10 @@ send_monitor_requests(struct shash *all_dbs)
 
 for (int j = 0; j < n; j++) {
 struct ovsdb_table_schema *table = nodes[j]->data;
-size_t blacklist_item_len = strlen(db_name) +
-strlen(table->name) + 2;
-char* blacklist_item = xmalloc(blacklist_item_len);
-
-snprintf(blacklist_item, blacklist_item_len, "%s%s%s",
- db_name, ":", table->name);
 
 /* Check if table is not blacklisted. */
+char *blacklist_item = xasprintf(
+"%s%s%s", db_name, ":", table->name);
 if (!sset_contains(_blacklist, blacklist_item)) {
 add_monitored_table(table, monitor_request);
 }
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/4] ovsdb: Introduce OVSDB replication feature

[Ben Pfaff]

On Fri, Jun 24, 2016 at 04:44:03PM -0700, Ben Pfaff wrote:
> From: Mario Cabrera 
> 
> Replication is enabled by using the following option when starting the
> database server:
> 
> --sync-from=server
> 
> Where 'server' can take any form described in the ovsdb-client(1)
> manpage as an active connection. If this option is specified, the
> replication process is immediately started.
> 
> Signed-off-by: Mario Cabrera 

Thanks for the patch.

I noticed a few odd indentations and a strdup() that should have been an
xstrdup(), so I'm folding in the following incremental.

Most of our code doesn't do blocking RPC calls in the way that this code
does.  That's probably something to consider replacing by a state
machine in the future.

--8<--cut here-->8--

diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index d9e609e..669f475 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -105,7 +105,7 @@ replication_run(struct shash *all_dbs)
 void
 set_remote_ovsdb_server(const char *remote_server)
 {
-remote_ovsdb_server = remote_server ? strdup(remote_server) : NULL;
+remote_ovsdb_server = remote_server ? xstrdup(remote_server) : NULL;
 }
 
 void
@@ -125,7 +125,7 @@ find_db(const struct shash *all_dbs, const char *db_name)
 {
 struct shash_node *node;
 
-SHASH_FOR_EACH(node, all_dbs) {
+SHASH_FOR_EACH (node, all_dbs) {
 struct db *db = node->data;
 if (!strcmp(db->db->schema->name, db_name)) {
 return db;
@@ -141,7 +141,7 @@ reset_databases(struct shash *all_dbs)
 struct shash_node *db_node;
 struct ovsdb_error *error = NULL;
 
-SHASH_FOR_EACH(db_node, all_dbs) {
+SHASH_FOR_EACH (db_node, all_dbs) {
 struct db *db = db_node->data;
 struct ovsdb_txn *txn = ovsdb_txn_create(db->db);
 reset_database(db->db, txn);
@@ -156,7 +156,7 @@ reset_database(struct ovsdb *db, struct ovsdb_txn *txn)
 {
 struct shash_node *table_node;
 
-SHASH_FOR_EACH(table_node, >tables) {
+SHASH_FOR_EACH (table_node, >tables) {
 struct ovsdb_table *table = table_node->data;
 struct ovsdb_row *row;
 
@@ -173,7 +173,7 @@ open_jsonrpc(const char *server)
 int error;
 
 error = stream_open_block(jsonrpc_stream_open(server, ,
-  DSCP_DEFAULT), );
+  DSCP_DEFAULT), );
 
 return error ? NULL : jsonrpc_open(stream);
 }
@@ -225,9 +225,9 @@ fetch_dbs(struct jsonrpc *rpc, struct svec *dbs)
 
 if (name->type != JSON_STRING) {
 ovsdb_error_assert(ovsdb_error(
-   "list_dbs failed",
-   "list_dbs response %"PRIuSIZE" is not string",
-   i));
+   "list_dbs failed",
+   "list_dbs response %"PRIuSIZE" is not 
string",
+   i));
 }
 svec_add(dbs, name->u.string);
 }
@@ -289,7 +289,7 @@ send_monitor_requests(struct shash *all_dbs)
 monitor_request = json_object_create();
 size_t n = shash_count(_schema->tables);
 const struct shash_node **nodes = shash_sort(
- _schema->tables);
+_schema->tables);
 
 for (int j = 0; j < n; j++) {
 struct ovsdb_table_schema *table = nodes[j]->data;
@@ -298,13 +298,13 @@ send_monitor_requests(struct shash *all_dbs)
 free(nodes);
 
 /* Send monitor request. */
- monitor = json_array_create_3(
-  json_string_create(db_name),
-  json_string_create(db_name),
-  monitor_request);
- request = jsonrpc_create_request("monitor", monitor, NULL);
- jsonrpc_send(rpc, request);
- get_initial_db_state(database);
+monitor = json_array_create_3(
+json_string_create(db_name),
+json_string_create(db_name),
+monitor_request);
+request = jsonrpc_create_request("monitor", monitor, NULL);
+jsonrpc_send(rpc, request);
+get_initial_db_state(database);
 }
 ovsdb_schema_destroy(remote_schema);
 }
@@ -358,7 +358,7 @@ check_for_notifications(struct shash *all_dbs)
 }
 if (msg->type == JSONRPC_REQUEST && !strcmp(msg->method, "echo")) {
 jsonrpc_send(rpc, jsonrpc_create_reply(json_clone(msg->params),
- msg->id));
+   msg->id));
 } else if (msg->type == JSONRPC_NOTIFY
&& !strcmp(msg->method, "update")) {
 struct json *params = msg->params;
@@ -369,7 +369,7 

Re: [ovs-dev] [PATCH 1/4] docs: OVSDB replication design document

[Ben Pfaff]

On Fri, Jun 24, 2016 at 04:44:02PM -0700, Ben Pfaff wrote:
> From: Mario Cabrera 
> 
> The database replication functionality is designed to provide "fail
> over" characteristics. There are two participating databases, one of
> which is the "active" database and the other is the "stand by" database.
> Replication happens exclusively from the active to the stand by
> database.
> 
> This document explains how the replication functionality is implemented.
> 
> Signed-off-by: Mario Cabrera 

Thanks for the patch.

I noticed a few spelling errors and formatting that didn't quite match
what we usually do, so I'm folding in the following incremental.

diff --git a/Documentation/OVSDB-replication.md 
b/Documentation/OVSDB-replication.md
index 4a4eb5e..74c9500 100644
--- a/Documentation/OVSDB-replication.md
+++ b/Documentation/OVSDB-replication.md
@@ -3,27 +3,32 @@ OVSDB replication implementation
 
 Overview
 
-Given two Open vSwitch databases that have the same schema, OVSDB replication
-consists on maintaining these databases in the same state with one another,
-i.e each of the databases have the same contents at any given time even if they
-are not running in the same host. This document elaborates on the 
implementation
-details to provide this functionality.
+
+Given two Open vSwitch databases with the same schema, OVSDB
+replication keeps these databases in the same state, i.e. each of the
+databases have the same contents at any given time even if they are
+not running in the same host.  This document elaborates on the
+implementation details to provide this functionality.
 
 Terminology
 ===
--   Source of truth database: database whose content will be replicated to
-another database.
--   Active server: ovsdb-server providing RPC interface to the source of
-truth database.
--   Standby server: ovsdb-server providing RPC interface to the database that
-is not the source of truth.
+
+- Source of truth database: database whose content will be replicated
+  to another database.
+
+- Active server: ovsdb-server providing RPC interface to the source of
+  truth database.
+
+- Standby server: ovsdb-server providing RPC interface to the database
+  that is not the source of truth.
 
 Design
 ==
-The overall design of replication consist on one ovsdb-server (active server)
+
+The overall design of replication consists of one ovsdb-server (active server)
 communicating the state of its databases to another ovsdb-server
 (standby server) so that the latter keep its own databases in that same state.
-In order to achieve this, the standby server acts as a client of the active
+To achieve this, the standby server acts as a client of the active
 server, in the sense that it sends a monitor request to keep up to date with
 the changes in the active server databases. When a notification from the
 active server arrives, the standby server executes the necessary set of
@@ -43,6 +48,7 @@ databases. Below is the design represented as a diagram.
 
 Setting up the replication
 ==
+
 To initiate the replication process, the standby server must be executed
 indicating the location of the active server via the command line option
 "--sync-from=server", where server can take any form described in the
@@ -53,29 +59,37 @@ server responds.
 
 When sending a monitor request the standby server is doing the following:
 
-1. Erase the content of the databases for which it is providing a RPC 
interface.
+1. Erase the content of the databases for which it is providing a RPC
+interface.
+
 2. Open the jsonrpc channel to communicate with the active server.
+
 3. Fetch all the databases located in the active server.
-4. For each database with the same schema in both the active and standby
-servers: construct and send a monitor request message specifying the tables
-that will be monitored (i.e all the tables on the database except the ones
-blacklisted*).
-5. Set the standby database to the current state of the active database.
-
-Once the monitor request message is sent, the standby server will continuosly
-receive notifications of changes occuring to the tables specified in the
+
+4. For each database with the same schema in both the active and
+   standby servers: construct and send a monitor request message
+   specifying the tables that will be monitored (i.e all the tables on
+   the database except the ones blacklisted*).
+
+5. Set the standby database to the current state of the active
+   database.
+
+Once the monitor request message is sent, the standby server will continuously
+receive notifications of changes occurring to the tables specified in the
 request. The process of handling this notifications is detailed in the next
 section.
 
-*A set of tables that will be excluded from replication can be configure as a
-blacklist of tables via the command line option 
"--sync-exclude-tables=db:table[,db:table]...",
-where db corresponds to the 

[ovs-dev] [PATCH 3/4] ovsdb: Add table exclusion functionality to OVSDB replication

[Ben Pfaff]

From: Mario Cabrera 

A blacklist of tables that will be excluded from replication can be
specified by the following option:

--sync-exclude-tables=db:table[,db:table]…

Where 'table' corresponds to a table name, and 'db' corresponds to the
database name where the table resides.

Signed-off-by: Mario Cabrera 
---
 ovsdb/ovsdb-server.c   |   6 ++
 ovsdb/replication.c|  58 ++-
 ovsdb/replication.h|   1 +
 tests/automake.mk  |   1 +
 tests/ovsdb-replication.at | 174 +
 tests/ovsdb-server.at  |  56 +++
 tests/ovsdb.at |   1 +
 7 files changed, 294 insertions(+), 3 deletions(-)
 create mode 100644 tests/ovsdb-replication.at

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 650c164..1b9de19 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -1266,6 +1266,7 @@ parse_options(int *argcp, char **argvp[],
 OPT_BOOTSTRAP_CA_CERT,
 OPT_PEER_CA_CERT,
 OPT_SYNC_FROM,
+OPT_SYNC_EXCLUDE,
 VLOG_OPTION_ENUMS,
 DAEMON_OPTION_ENUMS
 };
@@ -1285,6 +1286,7 @@ parse_options(int *argcp, char **argvp[],
 {"certificate", required_argument, NULL, 'c'},
 {"ca-cert", required_argument, NULL, 'C'},
 {"sync-from",   required_argument, NULL, OPT_SYNC_FROM},
+{"sync-exclude-tables", required_argument, NULL, OPT_SYNC_EXCLUDE},
 {NULL, 0, NULL, 0},
 };
 char *short_options = ovs_cmdl_long_options_to_short_options(long_options);
@@ -1350,6 +1352,10 @@ parse_options(int *argcp, char **argvp[],
 connect_to_remote_server = true;
 break;
 
+case OPT_SYNC_EXCLUDE:
+set_tables_blacklist(optarg);
+break;
+
 case '?':
 exit(EXIT_FAILURE);
 
diff --git a/ovsdb/replication.c b/ovsdb/replication.c
index d9e609e..4ef9f4f 100644
--- a/ovsdb/replication.c
+++ b/ovsdb/replication.c
@@ -35,8 +35,10 @@
 static char *remote_ovsdb_server;
 static struct jsonrpc *rpc;
 static struct sset monitored_tables = SSET_INITIALIZER(_tables);
+static struct sset tables_blacklist = SSET_INITIALIZER(_blacklist);
 static bool reset_dbs = true;
 
+void replication_init(void);
 static struct jsonrpc *open_jsonrpc(const char *server);
 static struct ovsdb_error *check_jsonrpc_error(int error,
struct jsonrpc_msg **reply_);
@@ -73,6 +75,14 @@ static struct ovsdb_error *execute_update(struct ovsdb_txn 
*txn,
   struct json *new);
 
 void
+replication_init(void)
+{
+sset_init(_tables);
+sset_init(_blacklist);
+reset_dbs = true;
+}
+
+void
 replication_run(struct shash *all_dbs)
 {
 if (sset_is_empty(_tables) && remote_ovsdb_server) {
@@ -109,10 +119,30 @@ set_remote_ovsdb_server(const char *remote_server)
 }
 
 void
+set_tables_blacklist(const char *blacklist)
+{
+char *save_ptr = NULL;
+char *blacklist_item;
+
+replication_init();
+
+if (blacklist) {
+   char *t_blacklist = strdup(blacklist);
+for (blacklist_item = strtok_r(t_blacklist, ",", _ptr);
+ blacklist_item != NULL;
+ blacklist_item = strtok_r(NULL, ",", _ptr)) {
+sset_add(_blacklist, blacklist_item);
+}
+free(t_blacklist);
+}
+}
+
+void
 disconnect_remote_server(void)
 {
 jsonrpc_close(rpc);
 sset_destroy(_tables);
+sset_destroy(_blacklist);
 
 if (remote_ovsdb_server) {
 free(remote_ovsdb_server);
@@ -160,9 +190,20 @@ reset_database(struct ovsdb *db, struct ovsdb_txn *txn)
 struct ovsdb_table *table = table_node->data;
 struct ovsdb_row *row;
 
-HMAP_FOR_EACH (row, hmap_node, >rows) {
-ovsdb_txn_row_delete(txn, row);
+size_t blacklist_item_len = strlen(db->schema->name) +
+strlen(table_node->name) + 2;
+
+/* Do not reset if table is blacklisted. */
+char* blacklist_item = xmalloc(blacklist_item_len);
+snprintf(blacklist_item, blacklist_item_len, "%s%s%s",
+ db->schema->name, ":", table_node->name);
+
+if (!sset_contains(_blacklist, blacklist_item)) {
+HMAP_FOR_EACH (row, hmap_node, >rows) {
+ovsdb_txn_row_delete(txn, row);
+}
 }
+free(blacklist_item);
 }
 }
 
@@ -293,7 +334,18 @@ send_monitor_requests(struct shash *all_dbs)
 
 for (int j = 0; j < n; j++) {
 struct ovsdb_table_schema *table = nodes[j]->data;
-add_monitored_table(table, monitor_request);
+size_t blacklist_item_len = strlen(db_name) +
+strlen(table->name) + 2;
+char* blacklist_item = xmalloc(blacklist_item_len);
+
+snprintf(blacklist_item, 

[ovs-dev] [PATCH 4/4] ovsdb: Add unixctl commands for OVSDB replication

[Ben Pfaff]

From: Mario Cabrera 

Set and get the server to replicate from:

ovsdb-server/set-remote-ovsdb-server {server}
ovsdb-server/get-remote-ovsdb-server

Set and get the replicated table blacklist:

ovsdb-server/set-sync-excluded-tables {DB:table,...}
ovsdb-server/get-sync-excluded-tables

Connect to the configured server and start replication:

ovsdb-server/connect-remote-ovsdb-server

Disconnect from the remote server and stop replication, without dropping
the replicated data:

ovsdb-server/disconnect-remote-ovsdb-server

Signed-off-by: Mario Cabrera 
---
 ovsdb/ovsdb-server.1.in |  21 ++
 ovsdb/ovsdb-server.c|  97 +
 ovsdb/replication.c |  13 +++-
 ovsdb/replication.h |   3 +
 tests/ovsdb-server.at   | 188 
 5 files changed, 321 insertions(+), 1 deletion(-)

diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index 4bbbace..f21b734 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -188,6 +188,27 @@ again (with \fBovsdb\-server/add\-db\fR).
 Outputs a list of the currently configured databases added either through
 the command line or through the \fBovsdb\-server/add\-db\fR command.
 .
+.IP "\fBovsdb\-server/set\-remote\-ovsdb\-server \fIserver"
+Sets  the remote \fIserver\fR from which \fBovsdb\-server\fR connects through
+\fBovsdb\-server/connect\-remote\-ovsdb\-server\fR.
+.
+.IP "\fBovsdb\-server/get\-remote\-ovsdb\-server"
+Gets the remote server from which \fBovsdb\-server\fR is currently 
synchronizing
+its databases.
+.
+.IP "\fBovsdb\-server/connect\-remote\-ovsdb\-server"
+Causes \fBovsdb\-server\fR to synchronize its databases with the server
+specified by \fBovsdb\-server/set\-remote\-ovsdb\-server\fR.
+.
+.IP "\fBovsdb\-server/disconnect\-remote\-ovsdb\-server"
+Causes \fBovsdb\-server\fR to  stop  synchronizing  its  databases with a 
remote server.
+.
+.IP "\fBovsdb\-server/set\-sync\-excluded\-tables 
\fIdb\fB:\fItable\fR[\fB,\fIdb\fB:\fItable\fR]..."
+Sets the \fItable\fR whitin \fIdb\fR that will be excluded from 
synchronization.
+.
+.IP "\fBovsdb\-server/get\-sync\-excluded\-tables"
+Gets  the  tables  that are currently excluded from synchronization.
+.
 .so lib/vlog-unixctl.man
 .so lib/memory-unixctl.man
 .so lib/coverage-unixctl.man
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 1b9de19..c06e8fd 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -77,6 +77,12 @@ static unixctl_cb_func ovsdb_server_reconnect;
 static unixctl_cb_func ovsdb_server_perf_counters_clear;
 static unixctl_cb_func ovsdb_server_perf_counters_show;
 static unixctl_cb_func ovsdb_server_disable_monitor2;
+static unixctl_cb_func ovsdb_server_set_remote_ovsdb_server;
+static unixctl_cb_func ovsdb_server_get_remote_ovsdb_server;
+static unixctl_cb_func ovsdb_server_connect_remote_ovsdb_server;
+static unixctl_cb_func ovsdb_server_disconnect_remote_ovsdb_server;
+static unixctl_cb_func ovsdb_server_set_sync_excluded_tables;
+static unixctl_cb_func ovsdb_server_get_sync_excluded_tables;
 
 struct server_config {
 struct sset *remotes;
@@ -334,6 +340,19 @@ main(int argc, char *argv[])
 unixctl_command_register("ovsdb-server/perf-counters-clear", "", 0, 0,
  ovsdb_server_perf_counters_clear, NULL);
 
+unixctl_command_register("ovsdb-server/set-remote-ovsdb-server", "", 0, 1,
+  ovsdb_server_set_remote_ovsdb_server, NULL);
+unixctl_command_register("ovsdb-server/get-remote-ovsdb-server", "", 0, 0,
+  ovsdb_server_get_remote_ovsdb_server, NULL);
+unixctl_command_register("ovsdb-server/connect-remote-ovsdb-server", "", 
0, 0,
+  ovsdb_server_connect_remote_ovsdb_server, NULL);
+unixctl_command_register("ovsdb-server/disconnect-remote-ovsdb-server", 
"", 0, 0,
+  ovsdb_server_disconnect_remote_ovsdb_server, 
NULL);
+unixctl_command_register("ovsdb-server/set-sync-excluded-tables", "", 0, 1,
+  ovsdb_server_set_sync_excluded_tables, NULL);
+unixctl_command_register("ovsdb-server/get-sync-excluded-tables", "", 0, 0,
+  ovsdb_server_get_sync_excluded_tables, NULL);
+
 /* Simulate the behavior of OVS release prior to version 2.5 that
  * does not support the monitor2 method.  */
 unixctl_command_register("ovsdb-server/disable-monitor2", "", 0, 0,
@@ -1019,6 +1038,84 @@ report_error_if_changed(char *error, char **last_errorp)
 }
 
 static void
+ovsdb_server_set_remote_ovsdb_server(struct unixctl_conn *conn,
+ int argc OVS_UNUSED, const char *argv[],
+ void *arg_ OVS_UNUSED)
+{
+set_remote_ovsdb_server(argv[1]);
+connect_to_remote_server = false;
+unixctl_command_reply(conn, 

[ovs-dev] [PATCH 1/4] docs: OVSDB replication design document

[Ben Pfaff]

From: Mario Cabrera 

The database replication functionality is designed to provide "fail
over" characteristics. There are two participating databases, one of
which is the "active" database and the other is the "stand by" database.
Replication happens exclusively from the active to the stand by
database.

This document explains how the replication functionality is implemented.

Signed-off-by: Mario Cabrera 
---
 Documentation/OVSDB-replication.md | 123 +
 Documentation/automake.mk  |   3 +-
 2 files changed, 125 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/OVSDB-replication.md

diff --git a/Documentation/OVSDB-replication.md 
b/Documentation/OVSDB-replication.md
new file mode 100644
index 000..4a4eb5e
--- /dev/null
+++ b/Documentation/OVSDB-replication.md
@@ -0,0 +1,123 @@
+OVSDB replication implementation
+
+
+Overview
+
+Given two Open vSwitch databases that have the same schema, OVSDB replication
+consists on maintaining these databases in the same state with one another,
+i.e each of the databases have the same contents at any given time even if they
+are not running in the same host. This document elaborates on the 
implementation
+details to provide this functionality.
+
+Terminology
+===
+-   Source of truth database: database whose content will be replicated to
+another database.
+-   Active server: ovsdb-server providing RPC interface to the source of
+truth database.
+-   Standby server: ovsdb-server providing RPC interface to the database that
+is not the source of truth.
+
+Design
+==
+The overall design of replication consist on one ovsdb-server (active server)
+communicating the state of its databases to another ovsdb-server
+(standby server) so that the latter keep its own databases in that same state.
+In order to achieve this, the standby server acts as a client of the active
+server, in the sense that it sends a monitor request to keep up to date with
+the changes in the active server databases. When a notification from the
+active server arrives, the standby server executes the necessary set of
+operations so its databases reach the same state as the the active server
+databases. Below is the design represented as a diagram.
+
++--+replication +--+
+|Active|<---|   Standby|
+| OVSDB-server || OVSDB-server |
++--++--+
+|  |
+|  |
++---+  +---+
+|  SoT  |  |   |
+| OVSDB |  | OVSDB |
++---+  +---+
+
+Setting up the replication
+==
+To initiate the replication process, the standby server must be executed
+indicating the location of the active server via the command line option
+"--sync-from=server", where server can take any form described in the
+ovsdb-client manpage and it must specify an active connection type (tcp, unix,
+ssl). This option will cause the standby server to attempt to send a monitor
+request to the active server in every main loop iteration, until the active
+server responds.
+
+When sending a monitor request the standby server is doing the following:
+
+1. Erase the content of the databases for which it is providing a RPC 
interface.
+2. Open the jsonrpc channel to communicate with the active server.
+3. Fetch all the databases located in the active server.
+4. For each database with the same schema in both the active and standby
+servers: construct and send a monitor request message specifying the tables
+that will be monitored (i.e all the tables on the database except the ones
+blacklisted*).
+5. Set the standby database to the current state of the active database.
+
+Once the monitor request message is sent, the standby server will continuosly
+receive notifications of changes occuring to the tables specified in the
+request. The process of handling this notifications is detailed in the next
+section.
+
+*A set of tables that will be excluded from replication can be configure as a
+blacklist of tables via the command line option 
"--sync-exclude-tables=db:table[,db:table]...",
+where db corresponds to the database where the table resides.
+
+Replication process
+===
+The replication proccess consists on handling the update notifications received
+in the standby server caused by the monitor request that was previously sent to
+the active server. In every loop interation, the standby server attempts to
+receive a message from the active server which can be an error, an echo
+message (used to keep the connection alive) or an update notification. In case
+the message is a fatal error, the standby 

[ovs-dev] [PATCH 2/4] ovsdb: Introduce OVSDB replication feature

[Ben Pfaff]

From: Mario Cabrera 

Replication is enabled by using the following option when starting the
database server:

--sync-from=server

Where 'server' can take any form described in the ovsdb-client(1)
manpage as an active connection. If this option is specified, the
replication process is immediately started.

Signed-off-by: Mario Cabrera 
---
 ovsdb/automake.mk |   6 +-
 ovsdb/ovsdb-server.1.in   |   3 +
 ovsdb/ovsdb-server.c  |  46 ++--
 ovsdb/replication-syn.man |   2 +
 ovsdb/replication.c   | 597 ++
 ovsdb/replication.h   |  39 +++
 ovsdb/replication.man |   8 +
 tests/ovsdb-server.at |  51 
 8 files changed, 725 insertions(+), 27 deletions(-)
 create mode 100644 ovsdb/replication-syn.man
 create mode 100644 ovsdb/replication.c
 create mode 100644 ovsdb/replication.h
 create mode 100644 ovsdb/replication.man

diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk
index 7db6fea..099ed3c 100644
--- a/ovsdb/automake.mk
+++ b/ovsdb/automake.mk
@@ -24,6 +24,8 @@ ovsdb_libovsdb_la_SOURCES = \
ovsdb/monitor.h \
ovsdb/query.c \
ovsdb/query.h \
+   ovsdb/replication.c \
+   ovsdb/replication.h \
ovsdb/row.c \
ovsdb/row.h \
ovsdb/server.c \
@@ -42,7 +44,9 @@ pkgconfig_DATA += \
 
 MAN_FRAGMENTS += \
ovsdb/remote-active.man \
-   ovsdb/remote-passive.man
+   ovsdb/remote-passive.man \
+   ovsdb/replication.man \
+   ovsdb/replication-syn.man
 
 # ovsdb-tool
 bin_PROGRAMS += ovsdb/ovsdb-tool
diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index f348a3b..4bbbace 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -19,6 +19,7 @@ ovsdb\-server \- Open vSwitch database server
 .so lib/daemon-syn.man
 .so lib/service-syn.man
 .so lib/vlog-syn.man
+.so ovsdb/replication-syn.man
 .so lib/ssl-syn.man
 .so lib/ssl-bootstrap-syn.man
 .so lib/ssl-peer-ca-cert-syn.man
@@ -100,6 +101,8 @@ configured remotes.
 .so lib/service.man
 .SS "Logging Options"
 .so lib/vlog.man
+.SS "Syncing Options"
+.so ovsdb/replication.man
 .SS "Public Key Infrastructure Options"
 The options described below for configuring the SSL public key
 infrastructure accept a special syntax for obtaining their
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 0ec3053..650c164 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -42,6 +42,7 @@
 #include "ovsdb-error.h"
 #include "poll-loop.h"
 #include "process.h"
+#include "replication.h"
 #include "row.h"
 #include "simap.h"
 #include "shash.h"
@@ -59,15 +60,7 @@
 
 VLOG_DEFINE_THIS_MODULE(ovsdb_server);
 
-struct db {
-/* Initialized in main(). */
-char *filename;
-struct ovsdb_file *file;
-struct ovsdb *db;
-
-/* Only used by update_remote_status(). */
-struct ovsdb_txn *txn;
-};
+struct db;
 
 /* SSL configuration. */
 static char *private_key_file;
@@ -75,6 +68,9 @@ static char *certificate_file;
 static char *ca_cert_file;
 static bool bootstrap_ca_cert;
 
+/* Replication configuration. */
+static bool connect_to_remote_server;
+
 static unixctl_cb_func ovsdb_server_exit;
 static unixctl_cb_func ovsdb_server_compact;
 static unixctl_cb_func ovsdb_server_reconnect;
@@ -159,6 +155,10 @@ main_loop(struct ovsdb_jsonrpc_server *jsonrpc, struct 
shash *all_dbs,
 report_error_if_changed(reconfigure_ssl(all_dbs), _error);
 ovsdb_jsonrpc_server_run(jsonrpc);
 
+if (connect_to_remote_server) {
+ replication_run(all_dbs);
+}
+
 SHASH_FOR_EACH(node, all_dbs) {
 struct db *db = node->data;
 ovsdb_trigger_run(db->db, time_msec());
@@ -170,9 +170,9 @@ main_loop(struct ovsdb_jsonrpc_server *jsonrpc, struct 
shash *all_dbs,
 }
 }
 
-/* update Manager status(es) every 5 seconds */
+/* update Manager status(es) every 2.5 seconds */
 if (time_msec() >= status_timer) {
-status_timer = time_msec() + 5000;
+status_timer = time_msec() + 2500;
 update_remote_status(jsonrpc, remotes, all_dbs);
 }
 
@@ -351,6 +351,7 @@ main(int argc, char *argv[])
 sset_destroy();
 sset_destroy(_filenames);
 unixctl_server_destroy(unixctl);
+disconnect_remote_server();
 
 if (run_process && process_exited(run_process)) {
 int status = process_status(run_process);
@@ -434,21 +435,6 @@ open_db(struct server_config *config, const char *filename)
 return error;
 }
 
-static const struct db *
-find_db(const struct shash *all_dbs, const char *db_name)
-{
-struct shash_node *node;
-
-SHASH_FOR_EACH(node, all_dbs) {
-struct db *db = node->data;
-if (!strcmp(db->db->schema->name, db_name)) {
-return db;
-}
-}
-
-return NULL;
-}
-
 static char * OVS_WARN_UNUSED_RESULT
 parse_db_column__(const struct shash *all_dbs,
   const 

Re: [ovs-dev] [PATCH v4 16/17] conntrack: Track ICMP type and code.

[Joe Stringer]

On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
> From the connection tracker perspective, an ICMP connection is a tuple
> identified by source ip address, destination ip address and ICMP id.
>
> While this allows basic ICMP traffic (pings) to work, it doesn't take
> into account the icmp type: the connection tracker will allow
> requests/replies in any directions.
>
> This is improved by making the ICMP type and code part of the connection
> tuple.  An ICMP echo request packet from A to B, will create a
> connection that matches ICMP echo request from A to B and ICMP echo
> replies from B to A.  The same is done for timestamp and info
> request/replies, and for ICMPv6.
>
> A new modules conntrack-icmp is implemented, to allow only "request"
> types to create new connections.
>
> Also, since they're tracked in both userspace and kernel
> implementations, ICMP type and code are always printed in ct-dpif (a few
> testcase are updated as a consequence).
>
> Reported-by: Subramani Paramasivam 
> Signed-off-by: Daniele Di Proietto 

Subramani, have you tried out this patch since you originally reported
the issue?

Implementation looks fine to me. I assume the newly introduced tests
validate at least the echo request/response paths.

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

Re: [ovs-dev] [PATCH v4 05/17] conntrack: Periodically delete expired connections.

[Daniele Di Proietto]

Thanks for your comments Joe, replies inline



On 24/06/2016 15:29, "Joe Stringer"  wrote:

>On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
>> This commit adds a thread that periodically removes expired connections.
>>
>> The expiration time of a connection can be expressed by:
>>
>> expiration = now + timeout
>>
>> For each possible 'timeout' value (there aren't many) we keep a list.
>> When the expiration is updated, we move the connection to the back of the
>> corresponding 'timeout' list. This ways, the list is always ordered by
>> 'expiration'.
>>
>> When the cleanup thread iterates through the lists for expired
>> connections, it can stop at the first non expired connection.
>>
>> Suggested-by: Joe Stringer 
>> Signed-off-by: Daniele Di Proietto 
>> ---
>>  lib/conntrack-other.c   |  11 +--
>>  lib/conntrack-private.h |  21 --
>>  lib/conntrack-tcp.c |  20 +++---
>>  lib/conntrack.c | 184 
>> 
>>  lib/conntrack.h |  32 -
>>  5 files changed, 237 insertions(+), 31 deletions(-)
>>
>> diff --git a/lib/conntrack-other.c b/lib/conntrack-other.c
>> index 295cb2c..2920889 100644
>> --- a/lib/conntrack-other.c
>> +++ b/lib/conntrack-other.c
>> @@ -43,8 +43,8 @@ conn_other_cast(const struct conn *conn)
>>  }
>>
>>  static enum ct_update_res
>> -other_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
>> -  bool reply, long long now)
>> +other_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
>> +  struct dp_packet *pkt OVS_UNUSED, bool reply, long long 
>> now)
>>  {
>>  struct conn_other *conn = conn_other_cast(conn_);
>>
>> @@ -54,7 +54,7 @@ other_conn_update(struct conn *conn_, struct dp_packet 
>> *pkt OVS_UNUSED,
>>  conn->state = OTHERS_MULTIPLE;
>>  }
>>
>> -update_expiration(conn_, other_timeouts[conn->state], now);
>> +conn_update_expiration(ctb, >up, othler_timeouts[conn->state], 
>> now);
>>
>>  return CT_UPDATE_VALID;
>>  }
>> @@ -66,14 +66,15 @@ other_valid_new(struct dp_packet *pkt OVS_UNUSED)
>>  }
>>
>>  static struct conn *
>> -other_new_conn(struct dp_packet *pkt OVS_UNUSED, long long now)
>> +other_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt 
>> OVS_UNUSED,
>> +   long long now)
>>  {
>>  struct conn_other *conn;
>>
>>  conn = xzalloc(sizeof *conn);
>>  conn->state = OTHERS_FIRST;
>>
>> -update_expiration(>up, other_timeouts[conn->state], now);
>> +conn_init_expiration(ctb, >up, other_timeouts[conn->state], now);
>>
>>  return >up;
>>  }
>> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
>> index d3e0099..4743dc6 100644
>> --- a/lib/conntrack-private.h
>> +++ b/lib/conntrack-private.h
>> @@ -68,10 +68,13 @@ enum ct_update_res {
>>  };
>>
>>  struct ct_l4_proto {
>> -struct conn *(*new_conn)(struct dp_packet *pkt, long long now);
>> +struct conn *(*new_conn)(struct conntrack_bucket *, struct dp_packet 
>> *pkt,
>> + long long now);
>>  bool (*valid_new)(struct dp_packet *pkt);
>> -enum ct_update_res (*conn_update)(struct conn *conn, struct dp_packet 
>> *pkt,
>> -  bool reply, long long now);
>> +enum ct_update_res (*conn_update)(struct conn *conn,
>> +  struct conntrack_bucket *,
>> +  struct dp_packet *pkt, bool reply,
>> +  long long now);
>>  };
>>
>>  extern struct ct_l4_proto ct_proto_tcp;
>> @@ -80,9 +83,19 @@ extern struct ct_l4_proto ct_proto_other;
>>  extern long long ct_timeout_val[];
>>
>>  static inline void
>> -update_expiration(struct conn *conn, enum ct_timeout tm, long long now)
>> +conn_init_expiration(struct conntrack_bucket *ctb, struct conn *conn,
>> +enum ct_timeout tm, long long now)
>>  {
>>  conn->expiration = now + ct_timeout_val[tm];
>> +ovs_list_push_back(>exp_lists[tm], >exp_node);
>> +}
>> +
>> +static inline void
>> +conn_update_expiration(struct conntrack_bucket *ctb, struct conn *conn,
>> +   enum ct_timeout tm, long long now)
>> +{
>> +ovs_list_remove(>exp_node);
>> +conn_init_expiration(ctb, conn, tm, now);
>>  }
>>
>>  #endif /* conntrack-private.h */
>> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
>> index b574eeb..71eadc1 100644
>> --- a/lib/conntrack-tcp.c
>> +++ b/lib/conntrack-tcp.c
>> @@ -152,8 +152,8 @@ tcp_payload_length(struct dp_packet *pkt)
>>  }
>>
>>  static enum ct_update_res
>> -tcp_conn_update(struct conn* conn_, struct dp_packet *pkt, bool reply,
>> -long long now)
>> +tcp_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
>> +struct dp_packet *pkt, bool reply, long long now)
>>  {
>>  struct conn_tcp *conn = 

Re: [ovs-dev] [PATCH] datapath:backport: openvswitch: Add packet len info to upcall.

[pravin shelar]

On Fri, Jun 24, 2016 at 3:50 PM, William Tu  wrote:
> Upstream commit:
> commit b95e5928fcc76d156352570858abdea7b2628efd
> Author: William Tu 
> Date:   Mon Jun 20 07:26:17 2016 -0700
>
> The commit f2a4d086ed4c ("openvswitch: Add packet truncation support.")
> introduces packet truncation before sending to userspace upcall receiver.
> This patch passes up the skb->len before truncation so that the upcall
> receiver knows the original packet size. Potentially this will be used
> by sFlow, where OVS translates sFlow config header=N to a sample action,
> truncating packet to N byte in kernel datapath. Thus, only N bytes instead
> of full-packet size is copied from kernel to userspace, saving the
> kernel-to-userspace bandwidth.
>
> Signed-off-by: William Tu 
> Cc: Pravin Shelar 
> Acked-by: Pravin B Shelar 
> Signed-off-by: David S. Miller 
>
> Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/140135299
> Signed-off-by: William Tu 
> ---

looks good to me. Applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] netdev-provider: Apply batch object to netdev provider.

[William Tu]

This patch applies the packet batch object to the netdev providers,
including dummy, Linux, BSD, and DPDK.

Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/140135888
Signed-off-by: William Tu 
---
 lib/netdev-bsd.c  |  9 --
 lib/netdev-dpdk.c | 81 +--
 lib/netdev-dummy.c|  9 --
 lib/netdev-linux.c|  9 --
 lib/netdev-provider.h | 25 
 lib/netdev.c  |  6 ++--
 6 files changed, 72 insertions(+), 67 deletions(-)

diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 2e92d97..1b55b1a 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -618,12 +618,13 @@ netdev_rxq_bsd_recv_tap(struct netdev_rxq_bsd *rxq, 
struct dp_packet *buffer)
 }
 
 static int
-netdev_bsd_rxq_recv(struct netdev_rxq *rxq_, struct dp_packet **packets,
-int *c)
+netdev_bsd_rxq_recv(struct netdev_rxq *rxq_, struct dp_packet_batch *batch)
 {
 struct netdev_rxq_bsd *rxq = netdev_rxq_bsd_cast(rxq_);
 struct netdev *netdev = rxq->up.netdev;
 struct dp_packet *packet;
+struct dp_packet **packets = batch->packets;
+int *c = >count;
 ssize_t retval;
 int mtu;
 
@@ -681,10 +682,12 @@ netdev_bsd_rxq_drain(struct netdev_rxq *rxq_)
  */
 static int
 netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
-struct dp_packet **pkts, int cnt, bool may_steal)
+struct dp_packet_batch *batch, bool may_steal)
 {
 struct netdev_bsd *dev = netdev_bsd_cast(netdev_);
 const char *name = netdev_get_name(netdev_);
+int cnt = batch->count;
+struct dp_packet **okts = batch->packets;
 int error;
 int i;
 
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ed14a21..3b1e7fa 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1251,12 +1251,14 @@ netdev_dpdk_vhost_update_rx_counters(struct 
netdev_stats *stats,
  */
 static int
 netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
-   struct dp_packet **packets, int *c)
+   struct dp_packet_batch *batch)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(rxq->netdev);
 struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(dev);
 int qid = rxq->queue_id;
 struct ingress_policer *policer = netdev_dpdk_get_ingress_policer(dev);
+struct dp_packet **packets = batch->packets;
+int *c = >count;
 uint16_t nb_rx = 0;
 uint16_t dropped = 0;
 
@@ -1292,12 +1294,13 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq,
 }
 
 static int
-netdev_dpdk_rxq_recv(struct netdev_rxq *rxq, struct dp_packet **packets,
- int *c)
+netdev_dpdk_rxq_recv(struct netdev_rxq *rxq, struct dp_packet_batch *batch)
 {
 struct netdev_rxq_dpdk *rx = netdev_rxq_dpdk_cast(rxq);
 struct netdev_dpdk *dev = netdev_dpdk_cast(rxq->netdev);
 struct ingress_policer *policer = netdev_dpdk_get_ingress_policer(dev);
+struct dp_packet **packets = batch->packets;
+int *c = >count;
 int nb_rx;
 int dropped = 0;
 
@@ -1371,11 +1374,13 @@ netdev_dpdk_vhost_update_tx_counters(struct 
netdev_stats *stats,
 
 static void
 __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
- struct dp_packet **pkts, int cnt,
+ struct dp_packet_batch *batch,
  bool may_steal)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
 struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(dev);
+struct dp_packet **pkts = batch->packets;
+int cnt = batch->count;
 struct rte_mbuf **cur_pkts = (struct rte_mbuf **) pkts;
 unsigned int total_pkts = cnt;
 unsigned int qos_pkts = cnt;
@@ -1423,11 +1428,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
 
 out:
 if (may_steal) {
-int i;
-
-for (i = 0; i < total_pkts; i++) {
-dp_packet_delete(pkts[i]);
-}
+dp_packet_delete_batch(batch, may_steal);
 }
 }
 
@@ -1462,18 +1463,19 @@ dpdk_queue_pkts(struct netdev_dpdk *dev, int qid,
 
 /* Tx function. Transmit packets indefinitely */
 static void
-dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet **pkts,
-int cnt)
-OVS_NO_THREAD_SAFETY_ANALYSIS
+dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet_batch *batch)
+OVS_NO_THREAD_SAFETY_ANALYSIS
 {
 #if !defined(__CHECKER__) && !defined(_WIN32)
-const size_t PKT_ARRAY_SIZE = cnt;
+const size_t PKT_ARRAY_SIZE = batch->count;
 #else
 /* Sparse or MSVC doesn't like variable length array. */
 enum { PKT_ARRAY_SIZE = NETDEV_MAX_BURST };
 #endif
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
 struct rte_mbuf *mbufs[PKT_ARRAY_SIZE];
+struct dp_packet **pkts = batch->packets;
+int cnt = batch->count;
 int dropped = 0;
 int newcnt = 0;
 int i;
@@ -1517,7 +1519,12 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct 
dp_packet **pkts,
 

[ovs-dev] [PATCH V11] 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 needs to be configured with
"--enable-ft" command line argument [i.e. configure --enable-ft]

This instrumentation logs the tracing output in separate log files
namely func_trace_.log. It does not use VLOG mechanism for
logging as that will make the patch very complicated to avoid
recursion in the trace routine.

This feature starts dumping output, only in debug mode, which means
ovs-appctl -t  vlog/set any:any:dbg should be used to enable
this logging.

Currently, only ovn-northd, ovn-controller, vswitchd are instrumented.

It is intended to be used for debugging purposes.
Signed-off-by: Nirapada Ghosh 
---
 configure.ac  |  10 +++
 include/openvswitch/vlog.h|   1 +
 lib/vlog.c|  23 ++
 ovn/controller/automake.mk|   5 ++
 ovn/controller/ovn-controller.c   |  10 +++
 ovn/northd/automake.mk|   5 ++
 ovn/northd/ovn-northd.c   |  10 +++
 third-party/automake.mk   |   6 ++
 third-party/function_tracer.c | 143 ++
 third-party/generate_ft_report.py |  79 +
 utilities/automake.mk |   1 +
 vswitchd/automake.mk  |   5 ++
 vswitchd/ovs-vswitchd.c   |   9 +++
 13 files changed, 307 insertions(+)
 create mode 100644 third-party/function_tracer.c
 create mode 100644 third-party/generate_ft_report.py

diff --git a/configure.ac b/configure.ac
index 05d80d5..4abb2ea 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,
+[  --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/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
index de64cbd..2df8796 100644
--- a/include/openvswitch/vlog.h
+++ b/include/openvswitch/vlog.h
@@ -57,6 +57,7 @@ enum vlog_level {
 VLL_N_LEVELS
 };
 
+void __attribute__ ((no_instrument_function)) vlog_directory(char *, int);
 const char *vlog_get_level_name(enum vlog_level);
 enum vlog_level vlog_get_level_val(const char *name);
 
diff --git a/lib/vlog.c b/lib/vlog.c
index 30b5bc2..c6fefdb 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -1262,6 +1262,29 @@ vlog_should_drop(const struct vlog_module *module, enum 
vlog_level level,
 return false;
 }
 
+void __attribute__ ((no_instrument_function))
+vlog_directory(char *dir,int len)
+{
+int dir_len;
+if (log_file_name == NULL) {
+dir_len = strlen(ovs_logdir());
+if (dir_len > len) {
+*dir = '\0';
+}
+snprintf(dir, dir_len, "%s", ovs_logdir());
+} else {
+char *fname = strrchr(log_file_name,'/');
+if (fname) {
+   dir_len = strlen(log_file_name) - strlen(fname)+1;
+   if (dir_len > len) {
+   *dir = '\0';
+   } else {
+   snprintf(dir, dir_len, "%s", log_file_name);
+   }
+}
+}
+}
+
 void
 vlog_rate_limit(const struct vlog_module *module, enum vlog_level level,
 struct vlog_rate_limit *rl, const char *message, ...)
diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index cf57bbd..8e8f3bc 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -20,6 +20,11 @@ ovn_controller_ovn_controller_SOURCES = \
ovn/controller/ovn-controller.h \
ovn/controller/physical.c \
ovn/controller/physical.h
+
+if ENABLE_FT
+ovn_controller_ovn_controller_SOURCES += third-party/function_tracer.c
+endif
+
 ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la lib/libopenvswitch.la
 man_MANS += ovn/controller/ovn-controller.8
 EXTRA_DIST += ovn/controller/ovn-controller.8.xml
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 47e6824..c0bc9a0 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -61,11 +61,20 @@ static unixctl_cb_func ct_zone_list;
 
 #define DEFAULT_BRIDGE_NAME "br-int"
 
+static bool g_command_line_args_parsed = false;
 static void parse_options(int argc, char 

Re: [ovs-dev] [PATCH 0/4] Restore ovn-controller binding functionality.

[Ben Pfaff]

On Fri, Jun 24, 2016 at 04:26:24PM -0500, Ryan Moats wrote:
> Ben Pfaff  wrote on 06/24/2016 04:16:14 PM:
> > I applied it before I saw these messages.
> >
> > If it doesn't do the job, and there's no obvious further fix, it's just
> > one more revert, no big deal.
> 
> No worries - it does pass the new test case, which is good and we are
> running upstream rechecks to see if it does the trick.

That is very good.

> BTW, I acked the new test case, with a proviso, so getting that in would be
> nice, in case this doesn't fix things and we need to revert everything out.

I agree that we should still add a test case.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V2] ipfix: Export user specified virtual observation ID

[Wenyu Zhang]

Sorry for the stupid mistake.
I am working on it，and will verify it by running check.

发自我的 iPhone

> 在 2016年6月25日，上午4:43，Ben Pfaff  写道：
> 
>> On Fri, Jun 24, 2016 at 05:25:57AM -0700, Wenyu Zhang wrote:
>> In virtual network, users want more info about the virtual point to observe 
>> the traffic.
>> It should be a string to provide clear info, not a simple interger ID.
>> 
>> Introduce "other-config: virtual_obs_id" in IPFIX, which is a string 
>> configured by user.
>> Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the 
>> value. The entity is a
>> variable-length string.
>> 
>> Signed-off-by: Wenyu Zhang 
> 
> Hi Wenyu.
> 
> I reverted this commit because it dumps core in test 1048 "ofproto-dpif
> - Flow IPFIX sanity check" with the following backtrace.  It crashes
> the same way whether I use your original commit or my modified one.
> 
> #0  hmap_first_with_hash (hmap=, hmap=, 
>hash=) at ../lib/hmap.h:328
> #1  smap_find__ (smap=0x94, key=key@entry=0x817f7ab "virtual_obs_id", 
>key_len=14, hash=2537071222) at ../lib/smap.c:366
> #2  0x0812b9d7 in smap_get_node (smap=0x9738a276, 
>key=0x817f7ab "virtual_obs_id") at ../lib/smap.c:198
> #3  0x0812ba30 in smap_get (smap=0x94, key=0x817f7ab "virtual_obs_id")
>at ../lib/smap.c:189
> #4  0x08055a60 in bridge_configure_ipfix (br=)
>at ../vswitchd/bridge.c:1237
> #5  bridge_reconfigure (ovs_cfg=0x94) at ../vswitchd/bridge.c:666
> #6  0x080568d3 in bridge_run () at ../vswitchd/bridge.c:2972
> #7  0x0804c9dd in main (argc=10, argv=0xffd8b934)
>at ../vswitchd/ovs-vswitchd.c:112
> 
> When you're ready, please post a fixed version.  Please start from the
> version that I committed.
> 
> Before you post a patch, run the unit tests.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] datapath:backport: openvswitch: Add packet len info to upcall.

[William Tu]

Upstream commit:
commit b95e5928fcc76d156352570858abdea7b2628efd
Author: William Tu 
Date:   Mon Jun 20 07:26:17 2016 -0700

The commit f2a4d086ed4c ("openvswitch: Add packet truncation support.")
introduces packet truncation before sending to userspace upcall receiver.
This patch passes up the skb->len before truncation so that the upcall
receiver knows the original packet size. Potentially this will be used
by sFlow, where OVS translates sFlow config header=N to a sample action,
truncating packet to N byte in kernel datapath. Thus, only N bytes instead
of full-packet size is copied from kernel to userspace, saving the
kernel-to-userspace bandwidth.

Signed-off-by: William Tu 
Cc: Pravin Shelar 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Tested-at: https://travis-ci.org/williamtu/ovs-travis/builds/140135299
Signed-off-by: William Tu 
---
 datapath/datapath.c   | 13 -
 datapath/linux/compat/include/linux/openvswitch.h |  2 ++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 169..b02807e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -395,7 +395,8 @@ static size_t upcall_msg_size(const struct dp_upcall_info 
*upcall_info,
 {
size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
+ nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
-   + nla_total_size(ovs_key_attr_size()); /* OVS_PACKET_ATTR_KEY */
+   + nla_total_size(ovs_key_attr_size()) /* OVS_PACKET_ATTR_KEY */
+   + nla_total_size(sizeof(unsigned int)); /* OVS_PACKET_ATTR_LEN 
*/
 
/* OVS_PACKET_ATTR_USERDATA */
if (upcall_info->userdata)
@@ -530,6 +531,16 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
pad_packet(dp, user_skb);
}
 
+   /* Add OVS_PACKET_ATTR_LEN when packet is truncated */
+   if (cutlen > 0) {
+   if (nla_put_u32(user_skb, OVS_PACKET_ATTR_LEN,
+   skb->len)) {
+   err = -ENOBUFS;
+   goto out;
+   }
+   pad_packet(dp, user_skb);
+   }
+
/* Only reserve room for attribute header, packet data is added
 * in skb_zerocopy()
 */
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index b92d9ed..f1e80db 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -188,6 +188,7 @@ enum ovs_packet_cmd {
  * output port is actually a tunnel port. Contains the output tunnel key
  * extracted from the packet as nested %OVS_TUNNEL_KEY_ATTR_* attributes.
  * @OVS_PACKET_ATTR_MRU: Present for an %OVS_PACKET_CMD_ACTION and
+ * @OVS_PACKET_ATTR_LEN: Packet size before truncation.
  * %OVS_PACKET_ATTR_USERSPACE action specify the Maximum received fragment
  * size.
  *
@@ -207,6 +208,7 @@ enum ovs_packet_attr {
OVS_PACKET_ATTR_PROBE,  /* Packet operation is a feature probe,
   error logging should be suppressed. */
OVS_PACKET_ATTR_MRU,/* Maximum received IP fragment size. */
+   OVS_PACKET_ATTR_LEN,/* Packet size before truncation. */
__OVS_PACKET_ATTR_MAX
 };
 
-- 
2.5.0

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

Re: [ovs-dev] [PATCH v4 15/17] system-tests: Add ping through conntrack test.

[Joe Stringer]

On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
> Signed-off-by: Daniele Di Proietto 

Thanks, I didn't realise this basic test would actually be really
helpful if there's something fundamentally going wrong in a setup. One
minor comment below.

Acked-by: Joe Stringer 

> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> +
> +dnl Without this sleep, we get occasional failures due to the following 
> error:
> +dnl "connect: Cannot assign requested address"
> +sleep 2;

Commit c10840ff42da ("system-traffic: Wait for IPv6 connectivity.")
replaced all of these in the testsuite, could you also update this to
do the following?

OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2])
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 12/17] flow: Generate checksum in flow_compose().

[Joe Stringer]

On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
> This is useful to test the connection tracker, which performs checksum
> verification.
>
> Signed-off-by: Daniele Di Proietto 

The comment above flow_compose() that says that checksums are missing
might do with an update.

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

Re: [ovs-dev] [PATCH v4 11/17] flow: Fill udp_len in flow_compose_l4().

[Joe Stringer]

On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
> It will be used by connection tracking tests.
>
> Signed-off-by: Daniele Di Proietto 

LGTM. A little surprised this isn't part of another patch, is there
something particular you want to highlight with this patch?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v4 05/17] conntrack: Periodically delete expired connections.

[Joe Stringer]

On 10 June 2016 at 15:47, Daniele Di Proietto  wrote:
> This commit adds a thread that periodically removes expired connections.
>
> The expiration time of a connection can be expressed by:
>
> expiration = now + timeout
>
> For each possible 'timeout' value (there aren't many) we keep a list.
> When the expiration is updated, we move the connection to the back of the
> corresponding 'timeout' list. This ways, the list is always ordered by
> 'expiration'.
>
> When the cleanup thread iterates through the lists for expired
> connections, it can stop at the first non expired connection.
>
> Suggested-by: Joe Stringer 
> Signed-off-by: Daniele Di Proietto 
> ---
>  lib/conntrack-other.c   |  11 +--
>  lib/conntrack-private.h |  21 --
>  lib/conntrack-tcp.c |  20 +++---
>  lib/conntrack.c | 184 
> 
>  lib/conntrack.h |  32 -
>  5 files changed, 237 insertions(+), 31 deletions(-)
>
> diff --git a/lib/conntrack-other.c b/lib/conntrack-other.c
> index 295cb2c..2920889 100644
> --- a/lib/conntrack-other.c
> +++ b/lib/conntrack-other.c
> @@ -43,8 +43,8 @@ conn_other_cast(const struct conn *conn)
>  }
>
>  static enum ct_update_res
> -other_conn_update(struct conn *conn_, struct dp_packet *pkt OVS_UNUSED,
> -  bool reply, long long now)
> +other_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
> +  struct dp_packet *pkt OVS_UNUSED, bool reply, long long 
> now)
>  {
>  struct conn_other *conn = conn_other_cast(conn_);
>
> @@ -54,7 +54,7 @@ other_conn_update(struct conn *conn_, struct dp_packet *pkt 
> OVS_UNUSED,
>  conn->state = OTHERS_MULTIPLE;
>  }
>
> -update_expiration(conn_, other_timeouts[conn->state], now);
> +conn_update_expiration(ctb, >up, othler_timeouts[conn->state], 
> now);
>
>  return CT_UPDATE_VALID;
>  }
> @@ -66,14 +66,15 @@ other_valid_new(struct dp_packet *pkt OVS_UNUSED)
>  }
>
>  static struct conn *
> -other_new_conn(struct dp_packet *pkt OVS_UNUSED, long long now)
> +other_new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt 
> OVS_UNUSED,
> +   long long now)
>  {
>  struct conn_other *conn;
>
>  conn = xzalloc(sizeof *conn);
>  conn->state = OTHERS_FIRST;
>
> -update_expiration(>up, other_timeouts[conn->state], now);
> +conn_init_expiration(ctb, >up, other_timeouts[conn->state], now);
>
>  return >up;
>  }
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index d3e0099..4743dc6 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -68,10 +68,13 @@ enum ct_update_res {
>  };
>
>  struct ct_l4_proto {
> -struct conn *(*new_conn)(struct dp_packet *pkt, long long now);
> +struct conn *(*new_conn)(struct conntrack_bucket *, struct dp_packet 
> *pkt,
> + long long now);
>  bool (*valid_new)(struct dp_packet *pkt);
> -enum ct_update_res (*conn_update)(struct conn *conn, struct dp_packet 
> *pkt,
> -  bool reply, long long now);
> +enum ct_update_res (*conn_update)(struct conn *conn,
> +  struct conntrack_bucket *,
> +  struct dp_packet *pkt, bool reply,
> +  long long now);
>  };
>
>  extern struct ct_l4_proto ct_proto_tcp;
> @@ -80,9 +83,19 @@ extern struct ct_l4_proto ct_proto_other;
>  extern long long ct_timeout_val[];
>
>  static inline void
> -update_expiration(struct conn *conn, enum ct_timeout tm, long long now)
> +conn_init_expiration(struct conntrack_bucket *ctb, struct conn *conn,
> +enum ct_timeout tm, long long now)
>  {
>  conn->expiration = now + ct_timeout_val[tm];
> +ovs_list_push_back(>exp_lists[tm], >exp_node);
> +}
> +
> +static inline void
> +conn_update_expiration(struct conntrack_bucket *ctb, struct conn *conn,
> +   enum ct_timeout tm, long long now)
> +{
> +ovs_list_remove(>exp_node);
> +conn_init_expiration(ctb, conn, tm, now);
>  }
>
>  #endif /* conntrack-private.h */
> diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
> index b574eeb..71eadc1 100644
> --- a/lib/conntrack-tcp.c
> +++ b/lib/conntrack-tcp.c
> @@ -152,8 +152,8 @@ tcp_payload_length(struct dp_packet *pkt)
>  }
>
>  static enum ct_update_res
> -tcp_conn_update(struct conn* conn_, struct dp_packet *pkt, bool reply,
> -long long now)
> +tcp_conn_update(struct conn *conn_, struct conntrack_bucket *ctb,
> +struct dp_packet *pkt, bool reply, long long now)
>  {
>  struct conn_tcp *conn = conn_tcp_cast(conn_);
>  struct tcp_header *tcp = dp_packet_l4(pkt);
> @@ -319,18 +319,18 @@ tcp_conn_update(struct conn* conn_, struct dp_packet 
> *pkt, bool reply,
>
>  if (src->state >= CT_DPIF_TCPS_FIN_WAIT_2
>  && 

Re: [ovs-dev] [PATCH V2] ipfix: Export user specified virtual observation ID

[Wenyu Zhang]

Thanks for your catch.
Iam working on it.

发自我的 iPhone

> 在 2016年6月25日，上午4:55，William Tu  写道：
> 
> Hi Wenyu,
> 
> I was debugging a little bit and the issue is a NULL pointer deference
> of be_cfg at   virtual_obs_id = smap_get(_cfg->other_config,
> "virtual_obs_id");
> 
> Maybe adding if (valid_be_cfg) check before the deference? I will
> leave you to fix it. Also I hope you can add a test case to this case.
> 
> Regards,
> William
> 
> 
>> On Fri, Jun 24, 2016 at 1:42 PM, Ben Pfaff  wrote:
>>> On Fri, Jun 24, 2016 at 05:25:57AM -0700, Wenyu Zhang wrote:
>>> In virtual network, users want more info about the virtual point to observe 
>>> the traffic.
>>> It should be a string to provide clear info, not a simple interger ID.
>>> 
>>> Introduce "other-config: virtual_obs_id" in IPFIX, which is a string 
>>> configured by user.
>>> Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the 
>>> value. The entity is a
>>> variable-length string.
>>> 
>>> Signed-off-by: Wenyu Zhang 
>> 
>> Hi Wenyu.
>> 
>> I reverted this commit because it dumps core in test 1048 "ofproto-dpif
>> - Flow IPFIX sanity check" with the following backtrace.  It crashes
>> the same way whether I use your original commit or my modified one.
>> 
>> #0  hmap_first_with_hash (hmap=, hmap=,
>>hash=) at ../lib/hmap.h:328
>> #1  smap_find__ (smap=0x94, key=key@entry=0x817f7ab "virtual_obs_id",
>>key_len=14, hash=2537071222) at ../lib/smap.c:366
>> #2  0x0812b9d7 in smap_get_node (smap=0x9738a276,
>>key=0x817f7ab "virtual_obs_id") at ../lib/smap.c:198
>> #3  0x0812ba30 in smap_get (smap=0x94, key=0x817f7ab "virtual_obs_id")
>>at ../lib/smap.c:189
>> #4  0x08055a60 in bridge_configure_ipfix (br=)
>>at ../vswitchd/bridge.c:1237
>> #5  bridge_reconfigure (ovs_cfg=0x94) at ../vswitchd/bridge.c:666
>> #6  0x080568d3 in bridge_run () at ../vswitchd/bridge.c:2972
>> #7  0x0804c9dd in main (argc=10, argv=0xffd8b934)
>>at ../vswitchd/ovs-vswitchd.c:112
>> 
>> When you're ready, please post a fixed version.  Please start from the
>> version that I committed.
>> 
>> Before you post a patch, run the unit tests.
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev=CwIBaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=MJBX2NeoFFE5o-wzhi1dy7zWKBNufpc6ky5q7EaQJBo=YK-ciEyP0lB9loxNsFHuW5TCW0xNKneVS3R9D-CunQo=GYVyJZTPKYoaidcS5nQES3WHzwOjvINyEuv41aVqlTg=
>>  
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Update support for NAT and Geneve.

[Jesse Gross]

Thanks, applied to master.

On Fri, Jun 24, 2016 at 3:07 PM, Nithin Raju  wrote:
> Acked-by: Nithin Raju 
>
> -Original Message-
> From: dev  on behalf of Jesse Gross
> 
> Date: Friday, June 24, 2016 at 2:58 PM
> To: "dev@openvswitch.org" 
> Subject: [ovs-dev] [PATCH] FAQ: Update support for NAT and Geneve.
>
>>Signed-off-by: Jesse Gross 
>>---
>> FAQ.md | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>>diff --git a/FAQ.md b/FAQ.md
>>index cc4fdf6..9f3dc61 100644
>>--- a/FAQ.md
>>+++ b/FAQ.md
>>@@ -192,13 +192,13 @@ A: Open vSwitch supports different datapaths on
>>different platforms.  Each
>>
>> Feature   | Linux upstream | Linux OVS tree | Userspace |
>>Hyper-V |
>>
>>--|:--:|:--:|:-:|:
>>---:|
>>-NAT   |  4.6   |   NO   |NO |
>>NO|
>>+NAT   |  4.6   |   YES  |NO |
>>NO|
>> Connection tracking   |  4.3   |   YES  |NO |
>>PARTIAL |
>> Tunnel - LISP |  NO|   YES  |NO |
>>NO|
>> Tunnel - STT  |  NO|   YES  |NO |
>>YES   |
>> Tunnel - GRE  |  3.11  |   YES  |YES|
>>YES   |
>> Tunnel - VXLAN|  3.12  |   YES  |YES|
>>YES   |
>>-Tunnel - Geneve   |  3.18  |   YES  |YES|
>>NO|
>>+Tunnel - Geneve   |  3.18  |   YES  |YES|
>>YES   |
>> QoS - Policing|  YES   |   YES  |NO |
>>NO|
>> QoS - Shaping |  YES   |   YES  |NO |
>>NO|
>> sFlow |  YES   |   YES  |YES|
>>NO|
>>--
>>2.7.4
>>
>>___
>>dev mailing list
>>dev@openvswitch.org
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>>n_listinfo_dev=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=pN
>>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80=R1Cw0JSSmgY98wC-R8JHwlDyxd7owP
>>cextAf_gCLQJ0=mojtotreiXrvWX67TzvdANWiMQ_dygMslrr0r9n9B14=
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Update support for NAT and Geneve.

[Nithin Raju]

Acked-by: Nithin Raju 

-Original Message-
From: dev  on behalf of Jesse Gross

Date: Friday, June 24, 2016 at 2:58 PM
To: "dev@openvswitch.org" 
Subject: [ovs-dev] [PATCH] FAQ: Update support for NAT and Geneve.

>Signed-off-by: Jesse Gross 
>---
> FAQ.md | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/FAQ.md b/FAQ.md
>index cc4fdf6..9f3dc61 100644
>--- a/FAQ.md
>+++ b/FAQ.md
>@@ -192,13 +192,13 @@ A: Open vSwitch supports different datapaths on
>different platforms.  Each
> 
> Feature   | Linux upstream | Linux OVS tree | Userspace |
>Hyper-V |
> 
>--|:--:|:--:|:-:|:
>---:|
>-NAT   |  4.6   |   NO   |NO |
>NO|
>+NAT   |  4.6   |   YES  |NO |
>NO|
> Connection tracking   |  4.3   |   YES  |NO |
>PARTIAL |
> Tunnel - LISP |  NO|   YES  |NO |
>NO|
> Tunnel - STT  |  NO|   YES  |NO |
>YES   |
> Tunnel - GRE  |  3.11  |   YES  |YES|
>YES   |
> Tunnel - VXLAN|  3.12  |   YES  |YES|
>YES   |
>-Tunnel - Geneve   |  3.18  |   YES  |YES|
>NO|
>+Tunnel - Geneve   |  3.18  |   YES  |YES|
>YES   |
> QoS - Policing|  YES   |   YES  |NO |
>NO|
> QoS - Shaping |  YES   |   YES  |NO |
>NO|
> sFlow |  YES   |   YES  |YES|
>NO|
>-- 
>2.7.4
>
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=pN
>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80=R1Cw0JSSmgY98wC-R8JHwlDyxd7owP
>cextAf_gCLQJ0=mojtotreiXrvWX67TzvdANWiMQ_dygMslrr0r9n9B14= 

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

Re: [ovs-dev] [PATCH] ovn-sbctl: Change lport-(un)bind to lsp-(un)bind.

[Amitabha Biswas]

> On Jun 24, 2016, at 1:37 PM, Russell Bryant  wrote:
> 
> A previous commit changed the command names in ovn-nbctl from lport-* to
> lsp-*.  Change lport-bind and lport-unbind in ovn-sbctl to match.
> 
> Signed-off-by: Russell Bryant 
> ---
> ovn/utilities/ovn-sbctl.c| 12 ++--
> tests/ovn-controller-vtep.at | 10 +-
> tests/ovn-sbctl.at   |  4 ++--
> tutorial/ovn/env3/setup.sh   |  4 ++--
> tutorial/ovn/env4/setup2.sh  |  4 ++--
> tutorial/ovn/env5/setup.sh   |  8 
> 6 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
> index c0ee518..80afa96 100644
> --- a/ovn/utilities/ovn-sbctl.c
> +++ b/ovn/utilities/ovn-sbctl.c
> @@ -317,8 +317,8 @@ Chassis commands:\n\
>   and gateway_ports\n\
> \n\
> Port binding commands:\n\
> -  lport-bind LPORT CHASSISbind logical port LPORT to CHASSIS\n\
> -  lport-unbind LPORT  reset the port binding of logical port LPORT\n\
> +  lsp-bind PORT CHASSIS  bind logical port PORT to CHASSIS\n\
> +  lsp-unbind PORTreset the port binding of logical port PORT\n\
> \n\
> Logical flow commands:\n\
>   lflow-list [DATAPATH]   List logical flows for all or a single 
> datapath\n\
> @@ -602,7 +602,7 @@ cmd_chassis_del(struct ctl_context *ctx)
> }
> 
> static void
> -cmd_lport_bind(struct ctl_context *ctx)
> +cmd_lsp_bind(struct ctl_context *ctx)
> {
> struct sbctl_context *sbctl_ctx = sbctl_context_cast(ctx);
> bool may_exist = shash_find(>options, "--may-exist") != NULL;
> @@ -631,7 +631,7 @@ cmd_lport_bind(struct ctl_context *ctx)
> }
> 
> static void
> -cmd_lport_unbind(struct ctl_context *ctx)
> +cmd_lsp_unbind(struct ctl_context *ctx)
> {
> struct sbctl_context *sbctl_ctx = sbctl_context_cast(ctx);
> bool must_exist = !shash_find(>options, "--if-exists");
> @@ -1022,9 +1022,9 @@ static const struct ctl_command_syntax sbctl_commands[] 
> = {
>  "--if-exists", RW},
> 
> /* Port binding commands. */
> -{"lport-bind", 2, 2, "LPORT CHASSIS", pre_get_info, cmd_lport_bind, NULL,
> +{"lsp-bind", 2, 2, "PORT CHASSIS", pre_get_info, cmd_lsp_bind, NULL,
>  "--may-exist", RW},
> -{"lport-unbind", 1, 1, "LPORT", pre_get_info, cmd_lport_unbind, NULL,
> +{"lsp-unbind", 1, 1, "PORT", pre_get_info, cmd_lsp_unbind, NULL,
>  "--if-exists", RW},
> 
> /* Logical flow commands */
> diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
> index 4bbda73..c296f0e 100644
> --- a/tests/ovn-controller-vtep.at
> +++ b/tests/ovn-controller-vtep.at
> @@ -341,7 +341,7 @@ OVN_CONTROLLER_VTEP_START
> AT_CHECK([ovn-nbctl lsp-add br-test vif0])
> AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:02])
> AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
> -AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
> +AT_CHECK([ovn-sbctl lsp-bind vif0 ch0])
> 
> # creates the logical switch in vtep and adds the corresponding logical
> # port to 'br-test'.
> @@ -355,7 +355,7 @@ AT_CHECK([ovn-nbctl ls-add br-void])
> AT_CHECK([ovn-nbctl lsp-add br-void vif1])
> AT_CHECK([ovn-nbctl lsp-set-addresses vif1 f0:ab:cd:ef:01:02])
> AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
> -AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
> +AT_CHECK([ovn-sbctl lsp-bind vif1 ch1])
> OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
> 
> # checks Ucast_Macs_Remote creation.
> @@ -412,12 +412,12 @@ OVN_CONTROLLER_VTEP_START
> AT_CHECK([ovn-nbctl lsp-add br-test vif0])
> AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:02])
> AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
> -AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
> +AT_CHECK([ovn-sbctl lsp-bind vif0 ch0])
> 
> # creates another vif in the same logical switch with duplicate mac.
> AT_CHECK([ovn-nbctl lsp-add br-test vif1])
> AT_CHECK([ovn-nbctl lsp-set-addresses vif1 f0:ab:cd:ef:01:02])
> -AT_CHECK([ovn-sbctl lport-bind vif1 ch0])
> +AT_CHECK([ovn-sbctl lsp-bind vif1 ch0])
> 
> # creates the logical switch in vtep and adds the corresponding logical
> # port to 'br-test'.
> @@ -446,7 +446,7 @@ AT_CHECK([ovn-nbctl ls-add br-void])
> AT_CHECK([ovn-nbctl lsp-add br-void vif1])
> AT_CHECK([ovn-nbctl lsp-set-addresses vif1 f0:ab:cd:ef:01:02])
> AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
> -AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
> +AT_CHECK([ovn-sbctl lsp-bind vif1 ch1])
> OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
> 
> # creates another logical switch in vtep and adds the corresponding logical
> diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
> index 5232933..3b61d9d 100644
> --- a/tests/ovn-sbctl.at
> +++ b/tests/ovn-sbctl.at
> @@ -83,7 +83,7 @@ AT_CHECK([ovn-nbctl ls-add br-test])
> AT_CHECK([ovn-nbctl lsp-add br-test vif0])
> AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:02])
> AT_CHECK([ovn-sbctl chassis-add ch0 stt 1.2.3.5])
> -AT_CHECK([ovn-sbctl 

Re: [ovs-dev] [PATCH 2/2] datapath-windows: Address minor alignment issues in Stt code.

[Guru Shetty]

On 24 June 2016 at 14:57, Nithin Raju  wrote:

> Acked-by: Nithin Raju 
>
Thank you, applied!


>
> -Original Message-
> From: dev  on behalf of Yin Lin
> 
> Date: Friday, June 24, 2016 at 2:44 PM
> To: "dev@openvswitch.org" 
> Cc: Yin Lin 
> Subject: [ovs-dev] [PATCH 2/2] datapath-windows: Address minor
> alignment   issues in Stt code.
>
> >Signed-off-by: Yin Lin 
> >---
> > datapath-windows/ovsext/Stt.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
> >index 0bac5f2..5aaf6fe 100644
> >--- a/datapath-windows/ovsext/Stt.c
> >+++ b/datapath-windows/ovsext/Stt.c
> >@@ -875,7 +875,7 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
> > advanceCnt = hdrLen;
> >
> > ipHdr = NdisGetDataBuffer(curNb, sizeof *ipHdr, (PVOID) ,
> >-1 /*no align*/, 0);
> >+  1 /*no align*/, 0);
> > ASSERT(ipHdr);
> >
> > TCPHdr *tcp = (TCPHdr *)((PCHAR)ipHdr + ipHdr->ihl * 4);
> >--
> >2.8.0.windows.1
> >
> >___
> >dev mailing list
> >dev@openvswitch.org
> >
> https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
> >n_listinfo_dev=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=pN
> >HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80=U5RIiG8Ghkgc-E8QlKXvj9zXH4PB9W
> >C0i95_iPb0msw=BcX3Pe8t_rrBYPx3rJchcXjG5O9s3jxSXgv0-2p02fk=
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] FAQ: Update support for NAT and Geneve.

[Jesse Gross]

Signed-off-by: Jesse Gross 
---
 FAQ.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/FAQ.md b/FAQ.md
index cc4fdf6..9f3dc61 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -192,13 +192,13 @@ A: Open vSwitch supports different datapaths on different 
platforms.  Each
 
 Feature   | Linux upstream | Linux OVS tree | Userspace | Hyper-V |
 --|:--:|:--:|:-:|:---:|
-NAT   |  4.6   |   NO   |NO |   NO|
+NAT   |  4.6   |   YES  |NO |   NO|
 Connection tracking   |  4.3   |   YES  |NO | PARTIAL |
 Tunnel - LISP |  NO|   YES  |NO |   NO|
 Tunnel - STT  |  NO|   YES  |NO |   YES   |
 Tunnel - GRE  |  3.11  |   YES  |YES|   YES   |
 Tunnel - VXLAN|  3.12  |   YES  |YES|   YES   |
-Tunnel - Geneve   |  3.18  |   YES  |YES|   NO|
+Tunnel - Geneve   |  3.18  |   YES  |YES|   YES   |
 QoS - Policing|  YES   |   YES  |NO |   NO|
 QoS - Shaping |  YES   |   YES  |NO |   NO|
 sFlow |  YES   |   YES  |YES|   NO|
-- 
2.7.4

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

Re: [ovs-dev] [PATCH 2/2] datapath-windows: Address minor alignment issues in Stt code.

[Nithin Raju]

Acked-by: Nithin Raju 

-Original Message-
From: dev  on behalf of Yin Lin

Date: Friday, June 24, 2016 at 2:44 PM
To: "dev@openvswitch.org" 
Cc: Yin Lin 
Subject: [ovs-dev] [PATCH 2/2] datapath-windows: Address minor
alignment   issues in Stt code.

>Signed-off-by: Yin Lin 
>---
> datapath-windows/ovsext/Stt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
>index 0bac5f2..5aaf6fe 100644
>--- a/datapath-windows/ovsext/Stt.c
>+++ b/datapath-windows/ovsext/Stt.c
>@@ -875,7 +875,7 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
> advanceCnt = hdrLen;
> 
> ipHdr = NdisGetDataBuffer(curNb, sizeof *ipHdr, (PVOID) ,
>-1 /*no align*/, 0);
>+  1 /*no align*/, 0);
> ASSERT(ipHdr);
> 
> TCPHdr *tcp = (TCPHdr *)((PCHAR)ipHdr + ipHdr->ihl * 4);
>-- 
>2.8.0.windows.1
>
>___
>dev mailing list
>dev@openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=pN
>HQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80=U5RIiG8Ghkgc-E8QlKXvj9zXH4PB9W
>C0i95_iPb0msw=BcX3Pe8t_rrBYPx3rJchcXjG5O9s3jxSXgv0-2p02fk= 

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

Re: [ovs-dev] [PATCH v10][PATCH 1/2] datapath-windows: Add Geneve support

[Yin Lin]

Nope. Nithin, can you approve the second patch. It’s a minor whitespace 
alignment patch.

From: Guru Shetty [mailto:g...@ovn.org]
Sent: Friday, June 24, 2016 2:48 PM
To: Yin Lin 
Cc: ovs dev 
Subject: Re: [ovs-dev] [PATCH v10][PATCH 1/2] datapath-windows: Add Geneve 
support



On 24 June 2016 at 14:44, Yin Lin > 
wrote:
Signed-off-by: Yin Lin >
Thank you. I added Nithin's Ack and applied it.
Is the second patch of this series reviewed?

---
 
datapath-windows/automake.mk
   |   2 +
 datapath-windows/ovsext/Actions.c  |  72 ++-
 datapath-windows/ovsext/Debug.h|   1 +
 datapath-windows/ovsext/DpInternal.h   |  29 ++-
 datapath-windows/ovsext/Flow.c | 179 +++--
 datapath-windows/ovsext/Flow.h |   7 +
 datapath-windows/ovsext/Geneve.c   | 356 +
 datapath-windows/ovsext/Geneve.h   | 121 +++
 datapath-windows/ovsext/Util.h |   1 +
 datapath-windows/ovsext/Vport.c|  20 +-
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 11 files changed, 715 insertions(+), 75 deletions(-)
 create mode 100644 datapath-windows/ovsext/Geneve.c
 create mode 100644 datapath-windows/ovsext/Geneve.h

diff --git 
a/datapath-windows/automake.mk
 
b/datapath-windows/automake.mk
index c9af806..53fb5c5 100644
--- 
a/datapath-windows/automake.mk
+++ 
b/datapath-windows/automake.mk
@@ -68,6 +68,8 @@ EXTRA_DIST += \
datapath-windows/ovsext/Vport.h \
datapath-windows/ovsext/Vxlan.c \
datapath-windows/ovsext/Vxlan.h \
+   datapath-windows/ovsext/Geneve.c \
+   datapath-windows/ovsext/Geneve.h \
datapath-windows/ovsext/ovsext.inf \
datapath-windows/ovsext/ovsext.rc \
datapath-windows/ovsext/ovsext.vcxproj \
diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 7ac6bb7..722a2a8 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -33,6 +33,7 @@
 #include "User.h"
 #include "Vport.h"
 #include "Vxlan.h"
+#include "Geneve.h"

 #ifdef OVS_DBG_MOD
 #undef OVS_DBG_MOD
@@ -48,6 +49,8 @@ typedef struct _OVS_ACTION_STATS {
 UINT64 txVxlan;
 UINT64 rxStt;
 UINT64 txStt;
+UINT64 rxGeneve;
+UINT64 txGeneve;
 UINT64 flowMiss;
 UINT64 flowUserspace;
 UINT64 txTcp;
@@ -237,6 +240,9 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
 case OVS_VPORT_TYPE_VXLAN:
 ovsActionStats.rxVxlan++;
 break;
+case OVS_VPORT_TYPE_GENEVE:
+ovsActionStats.rxGeneve++;
+break;
 case OVS_VPORT_TYPE_GRE:
 ovsActionStats.rxGre++;
 break;
@@ -333,6 +339,9 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
 case OVS_VPORT_TYPE_STT:
 ovsActionStats.txStt++;
 break;
+case OVS_VPORT_TYPE_GENEVE:
+   ovsActionStats.txGeneve++;
+   break;
 }
 ovsFwdCtx->tunnelTxNic = dstVport;
 }
@@ -689,6 +698,11 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
  >tunKey, ovsFwdCtx->switchContext,
  >layers, );
 break;
+case OVS_VPORT_TYPE_GENEVE:
+status = OvsEncapGeneve(ovsFwdCtx->tunnelTxNic, ovsFwdCtx->curNbl,
+>tunKey, ovsFwdCtx->switchContext,
+>layers, );
+break;
 default:
 ASSERT(! "Tx: Unhandled tunnel type");
 }
@@ -767,6 +781,10 @@ OvsTunnelPortRx(OvsForwardingContext *ovsFwdCtx)
 dropReason = L"OVS-STT segment is cached";
 }
 break;
+case 

Re: [ovs-dev] [PATCH v10][PATCH 1/2] datapath-windows: Add Geneve support

[Guru Shetty]

On 24 June 2016 at 14:44, Yin Lin  wrote:

> Signed-off-by: Yin Lin 
>
Thank you. I added Nithin's Ack and applied it.
Is the second patch of this series reviewed?


> ---
>  datapath-windows/automake.mk   |   2 +
>  datapath-windows/ovsext/Actions.c  |  72 ++-
>  datapath-windows/ovsext/Debug.h|   1 +
>  datapath-windows/ovsext/DpInternal.h   |  29 ++-
>  datapath-windows/ovsext/Flow.c | 179 +++--
>  datapath-windows/ovsext/Flow.h |   7 +
>  datapath-windows/ovsext/Geneve.c   | 356
> +
>  datapath-windows/ovsext/Geneve.h   | 121 +++
>  datapath-windows/ovsext/Util.h |   1 +
>  datapath-windows/ovsext/Vport.c|  20 +-
>  datapath-windows/ovsext/ovsext.vcxproj |   2 +
>  11 files changed, 715 insertions(+), 75 deletions(-)
>  create mode 100644 datapath-windows/ovsext/Geneve.c
>  create mode 100644 datapath-windows/ovsext/Geneve.h
>
> diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
> index c9af806..53fb5c5 100644
> --- a/datapath-windows/automake.mk
> +++ b/datapath-windows/automake.mk
> @@ -68,6 +68,8 @@ EXTRA_DIST += \
> datapath-windows/ovsext/Vport.h \
> datapath-windows/ovsext/Vxlan.c \
> datapath-windows/ovsext/Vxlan.h \
> +   datapath-windows/ovsext/Geneve.c \
> +   datapath-windows/ovsext/Geneve.h \
> datapath-windows/ovsext/ovsext.inf \
> datapath-windows/ovsext/ovsext.rc \
> datapath-windows/ovsext/ovsext.vcxproj \
> diff --git a/datapath-windows/ovsext/Actions.c
> b/datapath-windows/ovsext/Actions.c
> index 7ac6bb7..722a2a8 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -33,6 +33,7 @@
>  #include "User.h"
>  #include "Vport.h"
>  #include "Vxlan.h"
> +#include "Geneve.h"
>
>  #ifdef OVS_DBG_MOD
>  #undef OVS_DBG_MOD
> @@ -48,6 +49,8 @@ typedef struct _OVS_ACTION_STATS {
>  UINT64 txVxlan;
>  UINT64 rxStt;
>  UINT64 txStt;
> +UINT64 rxGeneve;
> +UINT64 txGeneve;
>  UINT64 flowMiss;
>  UINT64 flowUserspace;
>  UINT64 txTcp;
> @@ -237,6 +240,9 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
>  case OVS_VPORT_TYPE_VXLAN:
>  ovsActionStats.rxVxlan++;
>  break;
> +case OVS_VPORT_TYPE_GENEVE:
> +ovsActionStats.rxGeneve++;
> +break;
>  case OVS_VPORT_TYPE_GRE:
>  ovsActionStats.rxGre++;
>  break;
> @@ -333,6 +339,9 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
>  case OVS_VPORT_TYPE_STT:
>  ovsActionStats.txStt++;
>  break;
> +case OVS_VPORT_TYPE_GENEVE:
> +   ovsActionStats.txGeneve++;
> +   break;
>  }
>  ovsFwdCtx->tunnelTxNic = dstVport;
>  }
> @@ -689,6 +698,11 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
>   >tunKey, ovsFwdCtx->switchContext,
>   >layers, );
>  break;
> +case OVS_VPORT_TYPE_GENEVE:
> +status = OvsEncapGeneve(ovsFwdCtx->tunnelTxNic, ovsFwdCtx->curNbl,
> +>tunKey,
> ovsFwdCtx->switchContext,
> +>layers, );
> +break;
>  default:
>  ASSERT(! "Tx: Unhandled tunnel type");
>  }
> @@ -767,6 +781,10 @@ OvsTunnelPortRx(OvsForwardingContext *ovsFwdCtx)
>  dropReason = L"OVS-STT segment is cached";
>  }
>  break;
> +case OVS_VPORT_TYPE_GENEVE:
> +status = OvsDecapGeneve(ovsFwdCtx->switchContext,
> ovsFwdCtx->curNbl,
> +>tunKey, );
> +break;
>  default:
>  OVS_LOG_ERROR("Rx: Unhandled tunnel type: %d\n",
>tunnelRxVport->ovsType);
> @@ -1233,57 +1251,6 @@ OvsActionMplsPush(OvsForwardingContext *ovsFwdCtx,
>  }
>
>  /*
> - *
> --
> - * OvsTunnelAttrToIPv4TunnelKey --
> - *  Convert tunnel attribute to OvsIPv4TunnelKey.
> - *
> --
> - */
> -static __inline NDIS_STATUS
> -OvsTunnelAttrToIPv4TunnelKey(PNL_ATTR attr,
> - OvsIPv4TunnelKey *tunKey)
> -{
> -   PNL_ATTR a;
> -   INT rem;
> -
> -   tunKey->attr[0] = 0;
> -   tunKey->attr[1] = 0;
> -   tunKey->attr[2] = 0;
> -   ASSERT(NlAttrType(attr) == OVS_KEY_ATTR_TUNNEL);
> -
> -   NL_ATTR_FOR_EACH_UNSAFE (a, rem, NlAttrData(attr),
> -NlAttrGetSize(attr)) {
> -  switch (NlAttrType(a)) {
> -  case OVS_TUNNEL_KEY_ATTR_ID:
> - tunKey->tunnelId = NlAttrGetBe64(a);
> - tunKey->flags |= OVS_TNL_F_KEY;
> - break;
> -  case OVS_TUNNEL_KEY_ATTR_IPV4_SRC:
> - tunKey->src 

Re: [ovs-dev] [PATCH 0/4] Fix IP refragmentation in upcall/execute path.

[Jesse Gross]

On Wed, Jun 22, 2016 at 6:00 PM, Joe Stringer  wrote:
> When the backport of ip_do_fragment() was done, it seemed as though the
> upstream version of this function could always be trusted, provided we ensure
> that we cannot trigger the ICMP response checks inside the upstream
> implementation. This seemed to be correct, based on the kernel module tests
> passing on various platforms. Unfortunately, in each of the kernel module
> testsuite tests that handle IP fragments, non-fragmented traffic would be sent
> through the datapath before the fragmented traffic is sent. As a result, the
> refragmentation path for packets coming from userspace was not well-tested.
>
> This series addresses a bug found in the datapath execute path where
> refragmentation is expected, but not currently carried out. Patches 1-2 are
> trivial updates for upstream changes to OVS, Patch 3 is the main chunk which
> backports the upstream ip_do_fragment() function and dependencies, and patch 4
> ensures that the kernel module tests will now send ip fragments through the
> upcall/execute path as well as the regular path.

Other than the minor changes that I already commented on (which are
pretty trivial), this all looks reasonable to me:

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

Re: [ovs-dev] [PATCH] netdev-dpdk: Fix using uninitialized link_status.

[Daniele Di Proietto]

On 24/06/2016 06:54, "Aaron Conole"  wrote:

>Ilya Maximets  writes:
>
>> 'rte_eth_link_get_nowait()' works only with physical ports.
>> In case of vhost-user port, 'link' will stay uninitialized and there
>> will be random messages in log about link status.
>>
>> Ex.:
>> |dpdk(dpdk_watchdog2)|DBG|Port -1 Link Up - speed 1 Mbps - full-duplex
>>
>> Fix that by calling 'check_link_status()' only for physical ports.
>>
>> Signed-off-by: Ilya Maximets 
>> ---
>
>LGTM
>
>Acked-by: Aaron Conole 

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

[ovs-dev] [PATCH v10][PATCH 1/2] datapath-windows: Add Geneve support

[Yin Lin]

Signed-off-by: Yin Lin 
---
 datapath-windows/automake.mk   |   2 +
 datapath-windows/ovsext/Actions.c  |  72 ++-
 datapath-windows/ovsext/Debug.h|   1 +
 datapath-windows/ovsext/DpInternal.h   |  29 ++-
 datapath-windows/ovsext/Flow.c | 179 +++--
 datapath-windows/ovsext/Flow.h |   7 +
 datapath-windows/ovsext/Geneve.c   | 356 +
 datapath-windows/ovsext/Geneve.h   | 121 +++
 datapath-windows/ovsext/Util.h |   1 +
 datapath-windows/ovsext/Vport.c|  20 +-
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 11 files changed, 715 insertions(+), 75 deletions(-)
 create mode 100644 datapath-windows/ovsext/Geneve.c
 create mode 100644 datapath-windows/ovsext/Geneve.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index c9af806..53fb5c5 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -68,6 +68,8 @@ EXTRA_DIST += \
datapath-windows/ovsext/Vport.h \
datapath-windows/ovsext/Vxlan.c \
datapath-windows/ovsext/Vxlan.h \
+   datapath-windows/ovsext/Geneve.c \
+   datapath-windows/ovsext/Geneve.h \
datapath-windows/ovsext/ovsext.inf \
datapath-windows/ovsext/ovsext.rc \
datapath-windows/ovsext/ovsext.vcxproj \
diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 7ac6bb7..722a2a8 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -33,6 +33,7 @@
 #include "User.h"
 #include "Vport.h"
 #include "Vxlan.h"
+#include "Geneve.h"
 
 #ifdef OVS_DBG_MOD
 #undef OVS_DBG_MOD
@@ -48,6 +49,8 @@ typedef struct _OVS_ACTION_STATS {
 UINT64 txVxlan;
 UINT64 rxStt;
 UINT64 txStt;
+UINT64 rxGeneve;
+UINT64 txGeneve;
 UINT64 flowMiss;
 UINT64 flowUserspace;
 UINT64 txTcp;
@@ -237,6 +240,9 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
 case OVS_VPORT_TYPE_VXLAN:
 ovsActionStats.rxVxlan++;
 break;
+case OVS_VPORT_TYPE_GENEVE:
+ovsActionStats.rxGeneve++;
+break;
 case OVS_VPORT_TYPE_GRE:
 ovsActionStats.rxGre++;
 break;
@@ -333,6 +339,9 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
 case OVS_VPORT_TYPE_STT:
 ovsActionStats.txStt++;
 break;
+case OVS_VPORT_TYPE_GENEVE:
+   ovsActionStats.txGeneve++;
+   break;
 }
 ovsFwdCtx->tunnelTxNic = dstVport;
 }
@@ -689,6 +698,11 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
  >tunKey, ovsFwdCtx->switchContext,
  >layers, );
 break;
+case OVS_VPORT_TYPE_GENEVE:
+status = OvsEncapGeneve(ovsFwdCtx->tunnelTxNic, ovsFwdCtx->curNbl,
+>tunKey, ovsFwdCtx->switchContext,
+>layers, );
+break;
 default:
 ASSERT(! "Tx: Unhandled tunnel type");
 }
@@ -767,6 +781,10 @@ OvsTunnelPortRx(OvsForwardingContext *ovsFwdCtx)
 dropReason = L"OVS-STT segment is cached";
 }
 break;
+case OVS_VPORT_TYPE_GENEVE:
+status = OvsDecapGeneve(ovsFwdCtx->switchContext, ovsFwdCtx->curNbl,
+>tunKey, );
+break;
 default:
 OVS_LOG_ERROR("Rx: Unhandled tunnel type: %d\n",
   tunnelRxVport->ovsType);
@@ -1233,57 +1251,6 @@ OvsActionMplsPush(OvsForwardingContext *ovsFwdCtx,
 }
 
 /*
- * --
- * OvsTunnelAttrToIPv4TunnelKey --
- *  Convert tunnel attribute to OvsIPv4TunnelKey.
- * --
- */
-static __inline NDIS_STATUS
-OvsTunnelAttrToIPv4TunnelKey(PNL_ATTR attr,
- OvsIPv4TunnelKey *tunKey)
-{
-   PNL_ATTR a;
-   INT rem;
-
-   tunKey->attr[0] = 0;
-   tunKey->attr[1] = 0;
-   tunKey->attr[2] = 0;
-   ASSERT(NlAttrType(attr) == OVS_KEY_ATTR_TUNNEL);
-
-   NL_ATTR_FOR_EACH_UNSAFE (a, rem, NlAttrData(attr),
-NlAttrGetSize(attr)) {
-  switch (NlAttrType(a)) {
-  case OVS_TUNNEL_KEY_ATTR_ID:
- tunKey->tunnelId = NlAttrGetBe64(a);
- tunKey->flags |= OVS_TNL_F_KEY;
- break;
-  case OVS_TUNNEL_KEY_ATTR_IPV4_SRC:
- tunKey->src = NlAttrGetBe32(a);
- break;
-  case OVS_TUNNEL_KEY_ATTR_IPV4_DST:
- tunKey->dst = NlAttrGetBe32(a);
- break;
-  case OVS_TUNNEL_KEY_ATTR_TOS:
- tunKey->tos = NlAttrGetU8(a);
- break;
-  case OVS_TUNNEL_KEY_ATTR_TTL:
- tunKey->ttl = NlAttrGetU8(a);
- break;
-  case OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT:
- tunKey->flags |= 

[ovs-dev] [PATCH 2/2] datapath-windows: Address minor alignment issues in Stt code.

[Yin Lin]

Signed-off-by: Yin Lin 
---
 datapath-windows/ovsext/Stt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
index 0bac5f2..5aaf6fe 100644
--- a/datapath-windows/ovsext/Stt.c
+++ b/datapath-windows/ovsext/Stt.c
@@ -875,7 +875,7 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
 advanceCnt = hdrLen;
 
 ipHdr = NdisGetDataBuffer(curNb, sizeof *ipHdr, (PVOID) ,
-1 /*no align*/, 0);
+  1 /*no align*/, 0);
 ASSERT(ipHdr);
 
 TCPHdr *tcp = (TCPHdr *)((PCHAR)ipHdr + ipHdr->ihl * 4);
-- 
2.8.0.windows.1

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

Re: [ovs-dev] [PATCH 3/4] compat: Backport ip_do_fragment().

[Joe Stringer]

On 24 June 2016 at 14:33, Jesse Gross  wrote:
> On Wed, Jun 22, 2016 at 6:00 PM, Joe Stringer  wrote:
>> diff --git a/datapath/linux/compat/include/net/ip.h 
>> b/datapath/linux/compat/include/net/ip.h
>> index 29a4d43a1151..581e912cf2dd 100644
>> --- a/datapath/linux/compat/include/net/ip.h
>> +++ b/datapath/linux/compat/include/net/ip.h
> [...]
>> +static inline void rpl_ip_options_fragment(struct sk_buff *skb)
>
> Is there any reason this isn't in ip_output.c like ip_copy_metadata()?
> They seem like somewhat analogous helper functions.

I can put it there. The definition wasn't originally in either of
these files, but the declaration is. Maybe it's clearer if I move it
to ip_output.c.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath-windows: Remove double semi-colon(; ) in Tunnel.c

[Guru Shetty]

On 24 June 2016 at 14:19, Sairam Venugopal  wrote:

> Found by inspection.
>
> Signed-off-by: Sairam Venugopal 
>
Applied, thanks!


> ---
>  datapath-windows/ovsext/Tunnel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/datapath-windows/ovsext/Tunnel.c
> b/datapath-windows/ovsext/Tunnel.c
> index 97d2020..e583468 100644
> --- a/datapath-windows/ovsext/Tunnel.c
> +++ b/datapath-windows/ovsext/Tunnel.c
> @@ -225,7 +225,7 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
>  OvsCompletionList completionList;
>  KIRQL irql;
>  ULONG SendFlags = NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP;
> -OVS_DATAPATH *datapath = >datapath;;
> +OVS_DATAPATH *datapath = >datapath;
>
>  ASSERT(gOvsSwitchContext);
>
> --
> 2.5.0.windows.1
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 3/4] compat: Backport ip_do_fragment().

[Jesse Gross]

On Wed, Jun 22, 2016 at 6:00 PM, Joe Stringer  wrote:
> diff --git a/datapath/linux/compat/include/net/ip.h 
> b/datapath/linux/compat/include/net/ip.h
> index 29a4d43a1151..581e912cf2dd 100644
> --- a/datapath/linux/compat/include/net/ip.h
> +++ b/datapath/linux/compat/include/net/ip.h
[...]
> +static inline void rpl_ip_options_fragment(struct sk_buff *skb)

Is there any reason this isn't in ip_output.c like ip_copy_metadata()?
They seem like somewhat analogous helper functions.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/4] compat: ipv4: Pass struct net through ip_fragment.

[Joe Stringer]

On 24 June 2016 at 14:00, Jesse Gross  wrote:
> On Wed, Jun 22, 2016 at 6:00 PM, Joe Stringer  wrote:
>> diff --git a/acinclude.m4 b/acinclude.m4
>> index 52d0209ab88a..8760948fcf9b 100644
>> --- a/acinclude.m4
>> +++ b/acinclude.m4
>> @@ -408,7 +408,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>>[OVS_DEFINE([HAVE_INET_GET_LOCAL_PORT_RANGE_USING_NET])])
>>OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_defrag.*net],
>>[OVS_DEFINE([HAVE_IP_DEFRAG_TAKES_NET])])
>> -  OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_do_fragment])
>> +  OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_do_fragment.*net],
>> +  [OVS_DEFINE([HAVE_IP_DO_FRAGMENT_USING_NET])])
>
> This might be a good use for Jarno's newly introduced
> OVS_FIND_PARAM_IFELSE macro.

True, I can update this.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/4] Restore ovn-controller binding functionality.

[Ryan Moats]

Ben Pfaff  wrote on 06/24/2016 04:16:14 PM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org, Russell Bryant 
> Date: 06/24/2016 04:16 PM
> Subject: Re: [PATCH 0/4] Restore ovn-controller binding functionality.
>
> On Fri, Jun 24, 2016 at 03:07:26PM -0500, Ryan Moats wrote:
> > Ben Pfaff  wrote on 06/24/2016 02:59:30 PM:
> >
> > > From: Ben Pfaff 
> > > To: Russell Bryant 
> > > Cc: dev@openvswitch.org, Ryan Moats/Omaha/IBM@IBMUS
> > > Date: 06/24/2016 03:00 PM
> > > Subject: Re: [PATCH 0/4] Restore ovn-controller binding
functionality.
> > >
> > > On Fri, Jun 24, 2016 at 03:52:07PM -0400, Russell Bryant wrote:
> > > > "Convert binding_run to incremental processing" caused a regression
> > > > in port binding handling.  This was seen in OpenStack CI.  The last
> > > > commit here adds a test case that would have caught the issue in
OVN.
> > > >
> > > > There are 3 reverts as two later patches conflicted with reverting
> > > > patch 3/4.  This is just to show what has to be rolled back to get
> > > > to where the added test in patch 4/4 will pass.
> > > >
> > > > Ryan says he has a possible fix for the issue, so we should pursue
> > > > that before merging the reverts.
> > >
> > > This is interesting.  I expected some minor fallout, but I did not
> > > expect it to break severely.  Incremental processing is hard.
> > >
> > > Ryan, will your patch be ready soon?  At a glance, I do not see it
yet.
> > >
> >
> > It is hard, but I have a patch that looks hopeful - I've tested it
> > by hand, I'm running it through existing UT first and then I'll add
> > Russell's test and run it through UT again and then post it. I'd say
> > ETA of 30 minutes or less...
>
> I applied it before I saw these messages.
>
> If it doesn't do the job, and there's no obvious further fix, it's just
> one more revert, no big deal.
>

No worries - it does pass the new test case, which is good and we are
running
upstream rechecks to see if it does the trick.

BTW, I acked the new test case, with a proviso, so getting that in would be
nice, in case this doesn't fix things and we need to revert everything out.

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

Re: [ovs-dev] [PATCH 4/4] ovn: Test that port state goes up and down.

[Ryan Moats]

Russell Bryant  wrote on 06/24/2016 02:52:11 PM:

> From: Russell Bryant 
> To: dev@openvswitch.org
> Cc: Ryan Moats/Omaha/IBM@IBMUS, b...@ovn.org, Russell Bryant

> Date: 06/24/2016 02:52 PM
> Subject: [PATCH 4/4] ovn: Test that port state goes up and down.
>
> Some previous commits broke ovn-controller binding handling such that
> ovn-controller never cleared out the chassis column of the Port_Binding
> table.  This broke OpenStack CI for OVN.  This patch adds an OVN test
> case that would have caught this issue.
>
> Signed-off-by: Russell Bryant 
> ---
>  tests/ovn.at | 46 ++
>  1 file changed, 46 insertions(+)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 297070c..5aa1fd6 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -3182,3 +3182,49 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>
>  AT_CLEANUP
> +
> +# 1 hypervisor, 1 port
> +# make sure that the port state is properly set to up and back down
> +# when created and deleted.
> +AT_SETUP([ovn -- port state up and down])
> +AT_KEYWORDS([ovn])
> +ovn_start
> +
> +ovn-nbctl ls-add ls1
> +ovn-nbctl lsp-add ls1 lp1
> +ovn-nbctl lsp-set-addresses lp1 unknown
> +
> +net_add n1
> +sim_add hv1
> +echo 1
> +as hv1 ovs-vsctl add-br br-phys
> +echo 2
> +as hv1 ovn_attach n1 br-phys 192.168.0.1
> +echo 3
> +
> +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])
> +echo 4
> +
> +as hv1 ovs-vsctl del-port br-int vif1
> +OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lp1` = xdown])
> +
> +echo 5
> +as hv1
> +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as ovn-nb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +as northd
> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
> +
> +as main
> +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +AT_CLEANUP
> --
> 2.7.4
>

With the changes that Russell has suggested (i.e.
removing the echo statements)...

Acked-by: Ryan Moats 

Having this would have prevented today's fire drill :)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] FW: Subject: [ovsdb-idl 1/1] Fix issues detected in Partial Map Update feature

[Ben Pfaff]

On Mon, Jun 20, 2016 at 03:15:07PM +, Lutz, Arnoldo wrote:
> Please find below a patch fixing a memory leak detected in partial map update 
> feature.
> It seems the original mail got lost and it hasn't been checked.

I applied this patch to master.

It had a cast that wasn't needed, so I removed it:

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 758f886..9b3e933 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2233,8 +2233,7 @@ ovsdb_idl_txn_extract_mutations(struct ovsdb_idl_row *row,
 } else if (row->old != NULL) {
 old_datum = >old[idx];
 } else {
-old_datum = CONST_CAST(struct ovsdb_datum*,
-ovsdb_datum_default(>type));
+old_datum = ovsdb_datum_default(>type);
 }
 
 del_set = json_array_create_empty();

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

Re: [ovs-dev] [PATCH 2/2] test: Add more pmd tests.

[Daniele Di Proietto]

On 23/06/2016 14:56, "Ben Pfaff"  wrote:

>On Tue, Jun 07, 2016 at 02:08:45PM -0700, Daniele Di Proietto wrote:
>> These tests stress the pmd thread and multiqueue handling in
>> dpif-netdev.
>> 
>> Signed-off-by: Daniele Di Proietto 
>
>I barely skimmed these tests.  I did run them, and they passed for me.
>
>Let me know if you want more detailed review.
>
>Acked-by: Ben Pfaff 

Thanks, I applied this to master
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] datapath-windows: Remove double semi-colon(; ) in Tunnel.c

[Sairam Venugopal]

Found by inspection.

Signed-off-by: Sairam Venugopal 
---
 datapath-windows/ovsext/Tunnel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c
index 97d2020..e583468 100644
--- a/datapath-windows/ovsext/Tunnel.c
+++ b/datapath-windows/ovsext/Tunnel.c
@@ -225,7 +225,7 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
 OvsCompletionList completionList;
 KIRQL irql;
 ULONG SendFlags = NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP;
-OVS_DATAPATH *datapath = >datapath;;
+OVS_DATAPATH *datapath = >datapath;
 
 ASSERT(gOvsSwitchContext);
 
-- 
2.5.0.windows.1

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

Re: [ovs-dev] [PATCH 1/2] netdev-dummy: Allow configuring the numa_id for testing purposes.

[Daniele Di Proietto]

On 23/06/2016 14:53, "Ben Pfaff"  wrote:

>On Tue, Jun 07, 2016 at 02:08:44PM -0700, Daniele Di Proietto wrote:
>> This commit introduces an (undocumented) option for dummy Interfaces to
>> specify a dummy numa_id, to which the device belongs.  It will be used
>> to test the pmd threads in dpif-netdev.
>> 
>> Signed-off-by: Daniele Di Proietto 
>
>Acked-by: Ben Pfaff 

Thanks for the review

>
>>  {
>> -return 0;
>> +struct netdev_dummy *netdev = netdev_dummy_cast(netdev_);
>> +int numa_id;
>> +
>> +ovs_mutex_lock(>mutex);
>> +numa_id = netdev->numa_id;
>> +ovs_mutex_unlock(>mutex);
>> +
>> +return numa_id;
>>  }
>
>This is a place where a mid-block declaration seems like a winner:
>
>{
>struct netdev_dummy *netdev = netdev_dummy_cast(netdev_);
>
>ovs_mutex_lock(>mutex);
>int numa_id = netdev->numa_id;
>ovs_mutex_unlock(>mutex);
>
>return numa_id;
>}

Good idea, changed, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/4] Restore ovn-controller binding functionality.

[Ben Pfaff]

On Fri, Jun 24, 2016 at 03:07:26PM -0500, Ryan Moats wrote:
> Ben Pfaff  wrote on 06/24/2016 02:59:30 PM:
> 
> > From: Ben Pfaff 
> > To: Russell Bryant 
> > Cc: dev@openvswitch.org, Ryan Moats/Omaha/IBM@IBMUS
> > Date: 06/24/2016 03:00 PM
> > Subject: Re: [PATCH 0/4] Restore ovn-controller binding functionality.
> >
> > On Fri, Jun 24, 2016 at 03:52:07PM -0400, Russell Bryant wrote:
> > > "Convert binding_run to incremental processing" caused a regression
> > > in port binding handling.  This was seen in OpenStack CI.  The last
> > > commit here adds a test case that would have caught the issue in OVN.
> > >
> > > There are 3 reverts as two later patches conflicted with reverting
> > > patch 3/4.  This is just to show what has to be rolled back to get
> > > to where the added test in patch 4/4 will pass.
> > >
> > > Ryan says he has a possible fix for the issue, so we should pursue
> > > that before merging the reverts.
> >
> > This is interesting.  I expected some minor fallout, but I did not
> > expect it to break severely.  Incremental processing is hard.
> >
> > Ryan, will your patch be ready soon?  At a glance, I do not see it yet.
> >
> 
> It is hard, but I have a patch that looks hopeful - I've tested it
> by hand, I'm running it through existing UT first and then I'll add
> Russell's test and run it through UT again and then post it. I'd say
> ETA of 30 minutes or less...

I applied it before I saw these messages.

If it doesn't do the job, and there's no obvious further fix, it's just
one more revert, no big deal.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] ovs-bugtool: Port to python3.

[Joe Stringer]

Fix python2-specific code in ovs-bugtool:
* python2 long() is the same as python2 int() and python3 int(). Convert
  the long() to int().
* raw_input() was renamed to input(). Use python-six's input() on python2.
* Drop lambda tuple unpacking, we can go back to regular lambda syntax.
* file() can be replaced with open().

Signed-off-by: Joe Stringer 
---
 utilities/bugtool/ovs-bugtool.in | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/utilities/bugtool/ovs-bugtool.in b/utilities/bugtool/ovs-bugtool.in
index cc18285f5759..9e85bc769302 100755
--- a/utilities/bugtool/ovs-bugtool.in
+++ b/utilities/bugtool/ovs-bugtool.in
@@ -34,6 +34,7 @@
 #
 
 from __future__ import print_function
+from six.moves import input
 
 import getopt
 import re
@@ -893,10 +894,10 @@ def load_plugins(just_capabilities=False, filter=None):
[PII_NO, PII_YES, PII_MAYBE, PII_IF_CUSTOMIZED]:
 pii = xmldoc.documentElement.getAttribute("pii")
 if xmldoc.documentElement.getAttribute("min_size") != '':
-min_size = long(
+min_size = int(
 xmldoc.documentElement.getAttribute("min_size"))
 if xmldoc.documentElement.getAttribute("max_size") != '':
-max_size = long(
+max_size = int(
 xmldoc.documentElement.getAttribute("max_size"))
 if xmldoc.documentElement.getAttribute("min_time") != '':
 min_time = int(xmldoc.documentElement.getAttribute("min_time"))
@@ -998,7 +999,7 @@ def make_tar(subdir, suffix, output_fd, output_file):
 s = os.stat(v['filename'])
 ti.mtime = s.st_mtime
 ti.size = s.st_size
-tf.addfile(ti, file(v['filename']))
+tf.addfile(ti, open(v['filename']))
 except:
 pass
 finally:
@@ -1059,7 +1060,7 @@ def make_inventory(inventory, subdir):
 s.setAttribute('uptime', commands.getoutput(UPTIME))
 document.getElementsByTagName(INVENTORY_XML_ROOT)[0].appendChild(s)
 
-map(lambda (k, v): inventory_entry(document, subdir, k, v),
+map(lambda k_v: inventory_entry(document, subdir, k_v[0], k_v[1]),
 inventory.items())
 return document.toprettyxml()
 
@@ -1176,7 +1177,7 @@ def prettyDict(d):
 
 
 def yes(prompt):
-yn = raw_input(prompt)
+yn = input(prompt)
 
 return len(yn) == 0 or yn.lower()[0] == 'y'
 
-- 
2.8.2

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

Re: [ovs-dev] [PATCH] datapath-windows: Handle memory allocation failure for event creation

[Sairam Venugopal]

Hi Alin,

Thanks for the review. We currently post only 1 event at a time. Hence the
return statement instead of continue.

The previous continue tries to match the current event to the right queue
based on the Type and Queue Mask.

Regard,
Sairam

On 6/22/16, 7:40 AM, "Alin Serdean" 
wrote:

>
>
>
>
>> -Mesaj original-
>
>> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Sairam
>
>> Venugopal
>
>> Trimis: Wednesday, June 22, 2016 2:54 AM
>
>> Către: dev@openvswitch.org
>
>> Subiect: [ovs-dev] [PATCH] datapath-windows: Handle memory allocation
>
>> failure for event creation
>
>> 
>
>> Release the lock and return if an event entry fails to get allocated.
>
>> 
>
>> Signed-off-by: Sairam Venugopal 
>
>> ---
>
>>  datapath-windows/ovsext/Event.c | 7 +++
>
>>  1 file changed, 7 insertions(+)
>
>> 
>
>> diff --git a/datapath-windows/ovsext/Event.c b/datapath-
>
>> windows/ovsext/Event.c index abf8f0d..f9bea7f 100644
>
>> --- a/datapath-windows/ovsext/Event.c
>
>> +++ b/datapath-windows/ovsext/Event.c
>
>> @@ -134,6 +134,13 @@ OvsPostEvent(POVS_EVENT_ENTRY event)
>
>> 
>
>>  elem = (POVS_EVENT_QUEUE_ELEM)OvsAllocateMemoryWithTag(
>
>>  sizeof(*elem), OVS_EVENT_POOL_TAG);
>
>> +
>
>> +if (elem == NULL) {
>
>> +OVS_LOG_WARN("Fail to allocate memory for event");
>
>> +OvsReleaseEventQueueLock();
>
>> +return;
>
>[Alin Gabriel Serdean: ] maybe continue; instead of return; future
>allocations may be successful, but maybe that is me. Otherwise
>
>Acked-by: Alin Gabriel Serdean 
>
>> +}
>
>> +
>
>>  RtlCopyMemory(>event, event, sizeof elem->event);
>
>>  InsertTailList(>elemList, >link);
>
>>  queue->numElems++;
>
>> --
>
>> 2.5.0.windows.1
>
>> 
>
>> ___
>
>> dev mailing list
>
>> dev@openvswitch.org
>
>> 
>>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm
>>an_listinfo_dev=CwIGaQ=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs=
>>Dcruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ=JtES1-c_iyhoUC8-DfZCeL2VT_p
>>yLYGKy2xihcEqHqM=-_axeQGoTqqfYlTbiCtsV-JTzAqOg9Sr19qVvoZXDyY=
>

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

Re: [ovs-dev] Fix port binding update on OVS port delete events

[Ryan Moats]

Ben Pfaff  wrote on 06/24/2016 04:12:27 PM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 06/24/2016 04:12 PM
> Subject: Re: [ovs-dev] Fix port binding update on OVS port delete events
>
> On Fri, Jun 24, 2016 at 03:39:28PM -0500, Ryan Moats wrote:
> > Patch "Convert binding_run to incremental processing." introduced
> > a bug where the port binding table is not correctly updated when
> > an OVS port is deleted.  Fix this by
> > - persisting the lport shash used to record OVS ports
> > - change get_local_iface_ids to return a bool indicating if
> >   the persisted lport shash has changed
> > - change port binding table processing from incremental to full
> >   if the persisted lport shash has changed
> >
> > Signed-off-by: Ryan Moats 
>
> I applied this, thanks.
>
> I hope this fixes the test breakage.
>

That's the next step...  Ryan
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Fix port binding update on OVS port delete events

[Ben Pfaff]

On Fri, Jun 24, 2016 at 03:39:28PM -0500, Ryan Moats wrote:
> Patch "Convert binding_run to incremental processing." introduced
> a bug where the port binding table is not correctly updated when
> an OVS port is deleted.  Fix this by
> - persisting the lport shash used to record OVS ports
> - change get_local_iface_ids to return a bool indicating if
>   the persisted lport shash has changed
> - change port binding table processing from incremental to full
>   if the persisted lport shash has changed
> 
> Signed-off-by: Ryan Moats 

I applied this, thanks.

I hope this fixes the test breakage.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] Getting error while configuring OVS 2.5.90(unstable release) with DPDK 16.4

[Daniele Di Proietto]

2016-06-23 9:40 GMT-07:00 Joe Stringer :

> On 23 June 2016 at 05:47, Thiyagu Subramanyam  wrote:
> > Hi All,
> >
> > I am trying to use Openvswitch in user space using DPDK. So that i
> need
> > to install DPDK first in user space and then to configure OVS with DPDK
> > which i installed in user space. I am following the INSTALL DPDK.md
> > document,  the link is as follows --->
> > https://github.com/openvswitch/ovs/blob/master/INSTALL.DPDK.md.
> >
> >While configuring OVS 2.5.0(stable release of OVS) with DPDK 16.4 no
> > errors are occurring whereas while configuring OVS 2.5.90(unstable
> release)
> > with DPDK 16.4 errors are arising. Does I required any patch file for
> > neglecting this error?
> >
> >   Why i am going for OVS 2.5.90(cloned from master branch) is, NAT
> feature
> > is supported in this version of OVS, which is confirmed by OVS Community.
>
> The userspace datapath (ie, dpdk) doesn't support stateful NAT.
>
> > I am setting the environment variables as below,
> >
> > export DPDK_DIR=/home/blue/dpdk-16.04
> > export DPDK_BUILD=/home/blue/dpdk-16.04/x86_64-native-linuxapp-gcc
> >
> > also I have installed all the dependencies like autoreconf, lib-tools
> etc.,
> > which is required for OVS installation.
> >
> > At the time of configuring this OVS 2.5.90 with DPDK 16.4, I am getting
> > following errors.
> >
> > checking whether gcc accepts -fno-strict-aliasing... yes
> > checking whether gcc accepts -Qunused-arguments... no
> > checking whether gcc accepts -Wno-unused... yes
> > checking whether gcc accepts -Wno-unused-parameter... yes
> > checking target hint for cgcc... x86_64
> > checking whether make has GNU make $(if) extension... yes
> > checking whether dpdk datapath is enabled... yes
> > checking for
> > /home/blue/dpdk-16.04/x86_64-native-linuxapp-gcc/include/rte_config.h...
> yes
> > configure: error: Could not find DPDK libraries in
> > /home/blue/dpdk-16.04/x86_64-native-linuxapp-gcc/lib
>
> Does this path exist? Is DPDK compiled here?
>

Did you install numa libraries? Now they're required to build OVS with DPDK.

I just applied a commit from Bhanuprakash that makes the error message more
explicit in that case

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

[ovs-dev] [Patch v9][PATCH 1/2] datapath-windows: Add Geneve support

[Yin Lin]

Signed-off-by: Yin Lin 
---
 datapath-windows/automake.mk   |   2 +
 datapath-windows/ovsext/Actions.c  |  72 ++-
 datapath-windows/ovsext/Debug.h|   1 +
 datapath-windows/ovsext/DpInternal.h   |  29 ++-
 datapath-windows/ovsext/Flow.c | 179 +++--
 datapath-windows/ovsext/Flow.h |   7 +
 datapath-windows/ovsext/Geneve.c   | 356 +
 datapath-windows/ovsext/Geneve.h   | 122 +++
 datapath-windows/ovsext/Util.h |   1 +
 datapath-windows/ovsext/Vport.c|  20 +-
 datapath-windows/ovsext/ovsext.vcxproj |   2 +
 11 files changed, 716 insertions(+), 75 deletions(-)
 create mode 100644 datapath-windows/ovsext/Geneve.c
 create mode 100644 datapath-windows/ovsext/Geneve.h

diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
index c9af806..53fb5c5 100644
--- a/datapath-windows/automake.mk
+++ b/datapath-windows/automake.mk
@@ -68,6 +68,8 @@ EXTRA_DIST += \
datapath-windows/ovsext/Vport.h \
datapath-windows/ovsext/Vxlan.c \
datapath-windows/ovsext/Vxlan.h \
+   datapath-windows/ovsext/Geneve.c \
+   datapath-windows/ovsext/Geneve.h \
datapath-windows/ovsext/ovsext.inf \
datapath-windows/ovsext/ovsext.rc \
datapath-windows/ovsext/ovsext.vcxproj \
diff --git a/datapath-windows/ovsext/Actions.c 
b/datapath-windows/ovsext/Actions.c
index 7ac6bb7..722a2a8 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -33,6 +33,7 @@
 #include "User.h"
 #include "Vport.h"
 #include "Vxlan.h"
+#include "Geneve.h"
 
 #ifdef OVS_DBG_MOD
 #undef OVS_DBG_MOD
@@ -48,6 +49,8 @@ typedef struct _OVS_ACTION_STATS {
 UINT64 txVxlan;
 UINT64 rxStt;
 UINT64 txStt;
+UINT64 rxGeneve;
+UINT64 txGeneve;
 UINT64 flowMiss;
 UINT64 flowUserspace;
 UINT64 txTcp;
@@ -237,6 +240,9 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
 case OVS_VPORT_TYPE_VXLAN:
 ovsActionStats.rxVxlan++;
 break;
+case OVS_VPORT_TYPE_GENEVE:
+ovsActionStats.rxGeneve++;
+break;
 case OVS_VPORT_TYPE_GRE:
 ovsActionStats.rxGre++;
 break;
@@ -333,6 +339,9 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
 case OVS_VPORT_TYPE_STT:
 ovsActionStats.txStt++;
 break;
+case OVS_VPORT_TYPE_GENEVE:
+   ovsActionStats.txGeneve++;
+   break;
 }
 ovsFwdCtx->tunnelTxNic = dstVport;
 }
@@ -689,6 +698,11 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
  >tunKey, ovsFwdCtx->switchContext,
  >layers, );
 break;
+case OVS_VPORT_TYPE_GENEVE:
+status = OvsEncapGeneve(ovsFwdCtx->tunnelTxNic, ovsFwdCtx->curNbl,
+>tunKey, ovsFwdCtx->switchContext,
+>layers, );
+break;
 default:
 ASSERT(! "Tx: Unhandled tunnel type");
 }
@@ -767,6 +781,10 @@ OvsTunnelPortRx(OvsForwardingContext *ovsFwdCtx)
 dropReason = L"OVS-STT segment is cached";
 }
 break;
+case OVS_VPORT_TYPE_GENEVE:
+status = OvsDecapGeneve(ovsFwdCtx->switchContext, ovsFwdCtx->curNbl,
+>tunKey, );
+break;
 default:
 OVS_LOG_ERROR("Rx: Unhandled tunnel type: %d\n",
   tunnelRxVport->ovsType);
@@ -1233,57 +1251,6 @@ OvsActionMplsPush(OvsForwardingContext *ovsFwdCtx,
 }
 
 /*
- * --
- * OvsTunnelAttrToIPv4TunnelKey --
- *  Convert tunnel attribute to OvsIPv4TunnelKey.
- * --
- */
-static __inline NDIS_STATUS
-OvsTunnelAttrToIPv4TunnelKey(PNL_ATTR attr,
- OvsIPv4TunnelKey *tunKey)
-{
-   PNL_ATTR a;
-   INT rem;
-
-   tunKey->attr[0] = 0;
-   tunKey->attr[1] = 0;
-   tunKey->attr[2] = 0;
-   ASSERT(NlAttrType(attr) == OVS_KEY_ATTR_TUNNEL);
-
-   NL_ATTR_FOR_EACH_UNSAFE (a, rem, NlAttrData(attr),
-NlAttrGetSize(attr)) {
-  switch (NlAttrType(a)) {
-  case OVS_TUNNEL_KEY_ATTR_ID:
- tunKey->tunnelId = NlAttrGetBe64(a);
- tunKey->flags |= OVS_TNL_F_KEY;
- break;
-  case OVS_TUNNEL_KEY_ATTR_IPV4_SRC:
- tunKey->src = NlAttrGetBe32(a);
- break;
-  case OVS_TUNNEL_KEY_ATTR_IPV4_DST:
- tunKey->dst = NlAttrGetBe32(a);
- break;
-  case OVS_TUNNEL_KEY_ATTR_TOS:
- tunKey->tos = NlAttrGetU8(a);
- break;
-  case OVS_TUNNEL_KEY_ATTR_TTL:
- tunKey->ttl = NlAttrGetU8(a);
- break;
-  case OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT:
- tunKey->flags |= 

[ovs-dev] [PATCH 2/2] datapath-windows: Address minor alignment issues in Stt code.

[Yin Lin]

Signed-off-by: Yin Lin 
---
 datapath-windows/ovsext/Stt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
index 0bac5f2..5aaf6fe 100644
--- a/datapath-windows/ovsext/Stt.c
+++ b/datapath-windows/ovsext/Stt.c
@@ -875,7 +875,7 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
 advanceCnt = hdrLen;
 
 ipHdr = NdisGetDataBuffer(curNb, sizeof *ipHdr, (PVOID) ,
-1 /*no align*/, 0);
+  1 /*no align*/, 0);
 ASSERT(ipHdr);
 
 TCPHdr *tcp = (TCPHdr *)((PCHAR)ipHdr + ipHdr->ihl * 4);
-- 
2.8.0.windows.1

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

Re: [ovs-dev] [PATCH 2/4] compat: ipv4: Pass struct net through ip_fragment.

[Jesse Gross]

On Wed, Jun 22, 2016 at 6:00 PM, Joe Stringer  wrote:
> diff --git a/acinclude.m4 b/acinclude.m4
> index 52d0209ab88a..8760948fcf9b 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -408,7 +408,8 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
>[OVS_DEFINE([HAVE_INET_GET_LOCAL_PORT_RANGE_USING_NET])])
>OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_defrag.*net],
>[OVS_DEFINE([HAVE_IP_DEFRAG_TAKES_NET])])
> -  OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_do_fragment])
> +  OVS_GREP_IFELSE([$KSRC/include/net/ip.h], [ip_do_fragment.*net],
> +  [OVS_DEFINE([HAVE_IP_DO_FRAGMENT_USING_NET])])

This might be a good use for Jarno's newly introduced
OVS_FIND_PARAM_IFELSE macro.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V2] ipfix: Export user specified virtual observation ID

[William Tu]

Hi Wenyu,

I was debugging a little bit and the issue is a NULL pointer deference
of be_cfg at   virtual_obs_id = smap_get(_cfg->other_config,
"virtual_obs_id");

Maybe adding if (valid_be_cfg) check before the deference? I will
leave you to fix it. Also I hope you can add a test case to this case.

Regards,
William


On Fri, Jun 24, 2016 at 1:42 PM, Ben Pfaff  wrote:
> On Fri, Jun 24, 2016 at 05:25:57AM -0700, Wenyu Zhang wrote:
>> In virtual network, users want more info about the virtual point to observe 
>> the traffic.
>> It should be a string to provide clear info, not a simple interger ID.
>>
>> Introduce "other-config: virtual_obs_id" in IPFIX, which is a string 
>> configured by user.
>> Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the 
>> value. The entity is a
>> variable-length string.
>>
>> Signed-off-by: Wenyu Zhang 
>
> Hi Wenyu.
>
> I reverted this commit because it dumps core in test 1048 "ofproto-dpif
> - Flow IPFIX sanity check" with the following backtrace.  It crashes
> the same way whether I use your original commit or my modified one.
>
> #0  hmap_first_with_hash (hmap=, hmap=,
> hash=) at ../lib/hmap.h:328
> #1  smap_find__ (smap=0x94, key=key@entry=0x817f7ab "virtual_obs_id",
> key_len=14, hash=2537071222) at ../lib/smap.c:366
> #2  0x0812b9d7 in smap_get_node (smap=0x9738a276,
> key=0x817f7ab "virtual_obs_id") at ../lib/smap.c:198
> #3  0x0812ba30 in smap_get (smap=0x94, key=0x817f7ab "virtual_obs_id")
> at ../lib/smap.c:189
> #4  0x08055a60 in bridge_configure_ipfix (br=)
> at ../vswitchd/bridge.c:1237
> #5  bridge_reconfigure (ovs_cfg=0x94) at ../vswitchd/bridge.c:666
> #6  0x080568d3 in bridge_run () at ../vswitchd/bridge.c:2972
> #7  0x0804c9dd in main (argc=10, argv=0xffd8b934)
> at ../vswitchd/ovs-vswitchd.c:112
>
> When you're ready, please post a fixed version.  Please start from the
> version that I committed.
>
> Before you post a patch, run the unit tests.
> ___
> 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] acinclude: check for numa library

[Daniele Di Proietto]

Thanks, this certainly makes the error more explicit.

Applied to master!

2016-06-20 2:53 GMT-07:00 Loftus, Ciara :

> >
> > Numa library is needed for NUMA aware vHost User functionality.
> > Incase of missing numa package, the OVS DPDK configuration fails with
> > "error: Could not find DPDK libraries in /TARGET/lib" though
> > the DPDK library is installed.
> >
> > This patch fixes this inappropriate error by checking for presence of
> > numa library and output an appropriate error message "error: unable to
> > find libnuma, install the dependency package" in case of missing package.
> >
> > Signed-off-by: Bhanuprakash Bodireddy
> > 
> > ---
> >  acinclude.m4 | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 3978980..fddd913 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -209,6 +209,8 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> >[AC_DEFINE([VHOST_CUSE], [1], [DPDK vhost-cuse support enabled,
> > vhost-user disabled.])
> > DPDK_EXTRA_LIB="-lfuse"])
> >
> > +AC_SEARCH_LIBS([get_mempolicy],[numa],[],[AC_MSG_ERROR([unable
> > to find libnuma, install the dependency package])])
> > +
> >  # On some systems we have to add -ldl to link with dpdk
> >  #
> >  # This code, at first, tries to link without -ldl (""),
> > --
> > 2.4.11
>
>
> Acked-by: Ciara Loftus 
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v1] Add new column compute_types to OVN_Southbound

[Russell Bryant]

On Fri, Jun 24, 2016 at 4:37 PM, Amitabha Biswas  wrote:

> On Jun 24, 2016, at 6:41 AM, Russell Bryant  wrote:
>
>
> On Wed, Jun 22, 2016 at 5:55 PM, Amitabha Biswas 
> wrote:
>
>> > On Jun 22, 2016, at 2:30 PM, Guru Shetty  wrote:
>> >
>> >
>> >
>> > On 21 June 2016 at 12:20, Amitabha Biswas  azbis...@gmail.com>> wrote:
>> > This patch allows a OVN hypervisor administator to specify the
>> > type(s) of non-distributed logical port, the hypervisor would
>> > prefer to support.
>> > ...
>> > The operator can set the preference in the using the external-id
>> > 'ovn-compute-types'. The default preference (when the external-id
>> > is not set) is that the hypervisor supports all non-distributed
>> > ports.
>> >
>> > ovs-vsctl set open_vswitch external-ids:ovn-compute-types="vif"
>> > ovs-vsctl set open_vswitch
>> external-ids:ovn-compute-types="gateway_router"
>> > ovs-vsctl set open_vswitch
>> external-ids:ovn-compute-types="vif,gateway_router”
>> > ...
>> > Note: It is possible that operator may choose to ignore the
>> > preference set by the hypervisor, so the ovn-controller does
>> > not verify that the ports it is hosting matches the its preference.
>> >
>> > Signed-off-by: Amitabha Biswas  abis...@us.ibm.com>>
>> >
>> > Is the idea that OpenStack OVN plugin will read from the SB database
>> about which chassis will host the gateway?
>> >
>>
>> Yes that is the idea. The OpenStack OVN plugin can read the list of
>> chassis that can host a gateway and assign chassis to Logical_Routers in
>> the NB database.
>>
>> Here is the corresponding WIP OpenStack patch:
>> https://review.openstack.org/#/c/332434/ <
>> https://review.openstack.org/#/c/332434/>
>>
>
> This also wouldn't be the first instance of the OpenStack plugin reading
> the Chassis table.
>
> We already read it to get the value of the ovn-bridge-mappings
> configuration. The OpenStack plugin needs to know which Chassis has
> connectivity to a given network name.
>
> There will probably be more ... there is an unresolved issue where our
> OpenStack plugin needs to know whether a given Chassis is using the DPDK or
> Linux datapath so it can tell the compute part of OpenStack (nova) what
> virtual interface type to use for VMs on that host.  I've tried to find
> other ways to resolve this, but it seems the simplest solution overall is
> to expose a bit more data in external_ids of Chassis.
>
> --
> Russell Bryant
>
>
> External_Ids works as well. The default can be left empty (and interpreted
> appropriately) by the OpenStack plugin. The external id can be be a
> straight copy of the string specified in the ovs-vsctl for e.g.
>
> ovs-vsctl set open_vswitch
> external-ids:neutron-compute-types=“vif,gateway_router”
> will result in following in the Chassis row.
> external_ids={neutron-compute-types=“vif,gateway_router”,…}
>
> If we are in agreement, I can push V2 of the patch.
>

I do think using external_ids on Chassis seems better than adding the
compute_types column.

I'm not sure I've totally concluded on the patch in general.  I completely
understand the need for the information.  OpenStack has to decide where to
place gateway ports.  We need a way for administrators to communicate some
policy about what nodes are candidates for that.  What I'm not sure about
is if this is too OpenStack specific.  Even if it is a bit OpenStack
specific, keeping it as external_id keys makes me feel less concerned about
it, at least.  :-)

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

Re: [ovs-dev] [PATCH] bridge: Fix crash in bridge_configure_ipfix().

[Daniele Di Proietto]

Please discard this, Ben just reverted the change that introduced this.



On 24/06/2016 13:46, "Daniele Di Proietto"  wrote:

>'be_cfg' may be NULL at that point.
>
>Found by testsuite.
>
>1051: ofproto-dpif - Flow IPFIX sanity check  FAILED
>(ofproto-dpif.at:6002)
>1052: ofproto-dpif - Flow IPFIX sanity check - tunnel set FAILED
>(ofproto-dpif.at:6041)
>
>0  0x006ae830 in hmap_first_with_hash (hmap=0x110,
>hash=2537071222) at ../lib/hmap.h:328
>1  0x006af492 in smap_find__ (smap=0x110, key=0x762eda
>"virtual_obs_id", key_len=14, hash=2537071222) at ../lib/smap.c:366
>2  0x006aef19 in smap_get_node (smap=0x110, key=0x762eda
>"virtual_obs_id") at ../lib/smap.c:198
>3  0x006aeeaf in smap_get (smap=0x110, key=0x762eda
>"virtual_obs_id") at ../lib/smap.c:189
>4  0x00579972 in bridge_configure_ipfix (br=0x1266500) at
>../vswitchd/bridge.c:1237
>5  0x005782cb in bridge_reconfigure (ovs_cfg=0x12b9a00) at
>../vswitchd/bridge.c:666
>6  0x0057e3e8 in bridge_run () at ../vswitchd/bridge.c:2972
>7  0x0058391f in main (argc=10, argv=0x7ffea2f6eb08) at
>../vswitchd/ovs-vswitchd.c:112
>
>Fixes: 337bebe91c94("ipfix: Export user specified virtual observation
>ID")
>
>Signed-off-by: Daniele Di Proietto 
>---
> vswitchd/bridge.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>index 19e9f2d..2a963fd 100644
>--- a/vswitchd/bridge.c
>+++ b/vswitchd/bridge.c
>@@ -1234,8 +1234,12 @@ bridge_configure_ipfix(struct bridge *br)
> opts->enable_tunnel_sampling = smap_get_bool(
>
> _cfg->ipfix->other_config,
>   "enable-tunnel-sampling", 
> true);
>-virtual_obs_id = smap_get(_cfg->other_config,
>-  "virtual_obs_id");
>+
>+virtual_obs_id = be_cfg
>+ ? smap_get(_cfg->other_config,
>+"virtual_obs_id")
>+ : NULL;
>+
> opts->virtual_obs_id = (virtual_obs_id
> ? xstrdup(virtual_obs_id)
> : NULL);
>-- 
>2.8.1
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] Fix port binding update on OVS port delete events

[Ryan Moats]

Ryan Moats/Omaha/IBM@IBMUS wrote on 06/24/2016 03:39:28 PM:

> From: Ryan Moats/Omaha/IBM@IBMUS
> To: dev@openvswitch.org
> Cc: b...@ovn.org, russ...@ovn.org, Ryan Moats/Omaha/IBM@IBMUS
> Date: 06/24/2016 03:39 PM
> Subject: [PATCH] Fix port binding update on OVS port delete events
>
> Patch "Convert binding_run to incremental processing." introduced
> a bug where the port binding table is not correctly updated when
> an OVS port is deleted.  Fix this by
> - persisting the lport shash used to record OVS ports
> - change get_local_iface_ids to return a bool indicating if
>   the persisted lport shash has changed
> - change port binding table processing from incremental to full
>   if the persisted lport shash has changed
>
> Signed-off-by: Ryan Moats 
> ---

Sorry for the delay - had one of the e2e tests decide to race on
me and so I spent the time making sure it wasn't another breakage.

Ben and Russell in case you didn't get it direct:
http://patchwork.ozlabs.org/patch/640417/

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

Re: [ovs-dev] [PATCH v1] Add new column compute_types to OVN_Southbound

[Amitabha Biswas]

> On Jun 24, 2016, at 6:41 AM, Russell Bryant  wrote:
> 
> 
> On Wed, Jun 22, 2016 at 5:55 PM, Amitabha Biswas  > wrote:
> > On Jun 22, 2016, at 2:30 PM, Guru Shetty  > > wrote:
> >
> >
> >
> > On 21 June 2016 at 12:20, Amitabha Biswas  >   > >> wrote:
> > This patch allows a OVN hypervisor administator to specify the
> > type(s) of non-distributed logical port, the hypervisor would
> > prefer to support.
> > ...
> > The operator can set the preference in the using the external-id
> > 'ovn-compute-types'. The default preference (when the external-id
> > is not set) is that the hypervisor supports all non-distributed
> > ports.
> >
> > ovs-vsctl set open_vswitch external-ids:ovn-compute-types="vif"
> > ovs-vsctl set open_vswitch external-ids:ovn-compute-types="gateway_router"
> > ovs-vsctl set open_vswitch 
> > external-ids:ovn-compute-types="vif,gateway_router”
> > ...
> > Note: It is possible that operator may choose to ignore the
> > preference set by the hypervisor, so the ovn-controller does
> > not verify that the ports it is hosting matches the its preference.
> >
> > Signed-off-by: Amitabha Biswas  >   > >>
> >
> > Is the idea that OpenStack OVN plugin will read from the SB database about 
> > which chassis will host the gateway?
> >
> 
> Yes that is the idea. The OpenStack OVN plugin can read the list of chassis 
> that can host a gateway and assign chassis to Logical_Routers in the NB 
> database.
> 
> Here is the corresponding WIP OpenStack patch:
> https://review.openstack.org/#/c/332434/ 
>  
>  >
> 
> This also wouldn't be the first instance of the OpenStack plugin reading the 
> Chassis table.
> 
> We already read it to get the value of the ovn-bridge-mappings configuration. 
> The OpenStack plugin needs to know which Chassis has connectivity to a given 
> network name.
> 
> There will probably be more ... there is an unresolved issue where our 
> OpenStack plugin needs to know whether a given Chassis is using the DPDK or 
> Linux datapath so it can tell the compute part of OpenStack (nova) what 
> virtual interface type to use for VMs on that host.  I've tried to find other 
> ways to resolve this, but it seems the simplest solution overall is to expose 
> a bit more data in external_ids of Chassis.
> 
> -- 
> Russell Bryant

External_Ids works as well. The default can be left empty (and interpreted 
appropriately) by the OpenStack plugin. The external id can be be a straight 
copy of the string specified in the ovs-vsctl for e.g.

ovs-vsctl set open_vswitch 
external-ids:neutron-compute-types=“vif,gateway_router”
will result in following in the Chassis row.
external_ids={neutron-compute-types=“vif,gateway_router”,…}

If we are in agreement, I can push V2 of the patch.

Amitabha

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

[ovs-dev] [PATCH] bridge: Fix crash in bridge_configure_ipfix().

[Daniele Di Proietto]

'be_cfg' may be NULL at that point.

Found by testsuite.

1051: ofproto-dpif - Flow IPFIX sanity check  FAILED
(ofproto-dpif.at:6002)
1052: ofproto-dpif - Flow IPFIX sanity check - tunnel set FAILED
(ofproto-dpif.at:6041)

0  0x006ae830 in hmap_first_with_hash (hmap=0x110,
hash=2537071222) at ../lib/hmap.h:328
1  0x006af492 in smap_find__ (smap=0x110, key=0x762eda
"virtual_obs_id", key_len=14, hash=2537071222) at ../lib/smap.c:366
2  0x006aef19 in smap_get_node (smap=0x110, key=0x762eda
"virtual_obs_id") at ../lib/smap.c:198
3  0x006aeeaf in smap_get (smap=0x110, key=0x762eda
"virtual_obs_id") at ../lib/smap.c:189
4  0x00579972 in bridge_configure_ipfix (br=0x1266500) at
../vswitchd/bridge.c:1237
5  0x005782cb in bridge_reconfigure (ovs_cfg=0x12b9a00) at
../vswitchd/bridge.c:666
6  0x0057e3e8 in bridge_run () at ../vswitchd/bridge.c:2972
7  0x0058391f in main (argc=10, argv=0x7ffea2f6eb08) at
../vswitchd/ovs-vswitchd.c:112

Fixes: 337bebe91c94("ipfix: Export user specified virtual observation
ID")

Signed-off-by: Daniele Di Proietto 
---
 vswitchd/bridge.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 19e9f2d..2a963fd 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1234,8 +1234,12 @@ bridge_configure_ipfix(struct bridge *br)
 opts->enable_tunnel_sampling = smap_get_bool(

_cfg->ipfix->other_config,
   "enable-tunnel-sampling", 
true);
-virtual_obs_id = smap_get(_cfg->other_config,
-  "virtual_obs_id");
+
+virtual_obs_id = be_cfg
+ ? smap_get(_cfg->other_config,
+"virtual_obs_id")
+ : NULL;
+
 opts->virtual_obs_id = (virtual_obs_id
 ? xstrdup(virtual_obs_id)
 : NULL);
-- 
2.8.1

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

Re: [ovs-dev] [PATCH V2] ipfix: Export user specified virtual observation ID

[Ben Pfaff]

On Fri, Jun 24, 2016 at 05:25:57AM -0700, Wenyu Zhang wrote:
> In virtual network, users want more info about the virtual point to observe 
> the traffic.
> It should be a string to provide clear info, not a simple interger ID.
> 
> Introduce "other-config: virtual_obs_id" in IPFIX, which is a string 
> configured by user.
> Introduce an enterprise IPFIX entity "virtualObsID"(898) to export the value. 
> The entity is a
> variable-length string.
> 
> Signed-off-by: Wenyu Zhang 

Hi Wenyu.

I reverted this commit because it dumps core in test 1048 "ofproto-dpif
- Flow IPFIX sanity check" with the following backtrace.  It crashes
the same way whether I use your original commit or my modified one.

#0  hmap_first_with_hash (hmap=, hmap=, 
hash=) at ../lib/hmap.h:328
#1  smap_find__ (smap=0x94, key=key@entry=0x817f7ab "virtual_obs_id", 
key_len=14, hash=2537071222) at ../lib/smap.c:366
#2  0x0812b9d7 in smap_get_node (smap=0x9738a276, 
key=0x817f7ab "virtual_obs_id") at ../lib/smap.c:198
#3  0x0812ba30 in smap_get (smap=0x94, key=0x817f7ab "virtual_obs_id")
at ../lib/smap.c:189
#4  0x08055a60 in bridge_configure_ipfix (br=)
at ../vswitchd/bridge.c:1237
#5  bridge_reconfigure (ovs_cfg=0x94) at ../vswitchd/bridge.c:666
#6  0x080568d3 in bridge_run () at ../vswitchd/bridge.c:2972
#7  0x0804c9dd in main (argc=10, argv=0xffd8b934)
at ../vswitchd/ovs-vswitchd.c:112

When you're ready, please post a fixed version.  Please start from the
version that I committed.

Before you post a patch, run the unit tests.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] Fix port binding update on OVS port delete events

[Ryan Moats]

Patch "Convert binding_run to incremental processing." introduced
a bug where the port binding table is not correctly updated when
an OVS port is deleted.  Fix this by
- persisting the lport shash used to record OVS ports
- change get_local_iface_ids to return a bool indicating if
  the persisted lport shash has changed
- change port binding table processing from incremental to full
  if the persisted lport shash has changed

Signed-off-by: Ryan Moats 
---
 ovn/controller/binding.c | 36 +---
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index 9921a49..fd01c71 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -59,10 +59,15 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
  _interface_col_ingress_policing_burst);
 }
 
-static void
+static bool
 get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lports)
 {
 int i;
+bool changed = false;
+
+/* A local copy of ports that we can use to compare with the persisted
+ * list. */
+struct shash local_ports = SHASH_INITIALIZER(_ports);
 
 for (i = 0; i < br_int->n_ports; i++) {
 const struct ovsrec_port *port_rec = br_int->ports[i];
@@ -81,13 +86,26 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, 
struct shash *lports)
 if (!iface_id) {
 continue;
 }
-shash_add(lports, iface_id, iface_rec);
+shash_add(_ports, iface_id, iface_rec);
+if (!shash_find(lports, iface_id)) {
+shash_add(lports, iface_id, iface_rec);
+changed = true;
+}
 if (!sset_find(_lports, iface_id)) {
 sset_add(_lports, iface_id);
 binding_reset_processing();
 }
 }
 }
+struct shash_node *iter, *next;
+SHASH_FOR_EACH_SAFE(iter, next, lports) {
+if (!shash_find_and_delete(_ports, iter->name)) {
+shash_delete(lports, iter);
+changed = true;
+}
+}
+shash_destroy(_ports);
+return changed;
 }
 
 /* Contains "struct local_datpath" nodes whose hash values are the
@@ -176,7 +194,7 @@ consider_local_datapath(struct controller_ctx *ctx, struct 
shash *lports,
 struct hmap *local_datapaths)
 {
 const struct ovsrec_interface *iface_rec
-= shash_find_and_delete(lports, binding_rec->logical_port);
+= shash_find_data(lports, binding_rec->logical_port);
 if (iface_rec
 || (binding_rec->parent_port && binding_rec->parent_port[0] &&
 sset_contains(_lports, binding_rec->parent_port))) {
@@ -221,6 +239,10 @@ consider_local_datapath(struct controller_ctx *ctx, struct 
shash *lports,
 }
 }
 
+/* We persist lports because we need to know when it changes to
+ * handle ports going away correctly in the binding record. */
+static struct shash lports = SHASH_INITIALIZER();
+
 void
 binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
 const char *chassis_id, struct hmap *local_datapaths)
@@ -233,12 +255,14 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 return;
 }
 
-struct shash lports = SHASH_INITIALIZER();
 if (br_int) {
-get_local_iface_ids(br_int, );
+if (get_local_iface_ids(br_int, )) {
+process_full_binding = true;
+}
 } else {
 /* We have no integration bridge, therefore no local logical ports.
  * We'll remove our chassis from all port binding records below. */
+process_full_binding = true;
 }
 
 /* Run through each binding record to see if it is resident on this
@@ -274,8 +298,6 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 }
 }
 }
-
-shash_destroy();
 }
 
 /* Returns true if the database is all cleaned up, false if more work is
-- 
1.9.1

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

[ovs-dev] [PATCH] ovn-sbctl: Change lport-(un)bind to lsp-(un)bind.

[Russell Bryant]

A previous commit changed the command names in ovn-nbctl from lport-* to
lsp-*.  Change lport-bind and lport-unbind in ovn-sbctl to match.

Signed-off-by: Russell Bryant 
---
 ovn/utilities/ovn-sbctl.c| 12 ++--
 tests/ovn-controller-vtep.at | 10 +-
 tests/ovn-sbctl.at   |  4 ++--
 tutorial/ovn/env3/setup.sh   |  4 ++--
 tutorial/ovn/env4/setup2.sh  |  4 ++--
 tutorial/ovn/env5/setup.sh   |  8 
 6 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index c0ee518..80afa96 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -317,8 +317,8 @@ Chassis commands:\n\
   and gateway_ports\n\
 \n\
 Port binding commands:\n\
-  lport-bind LPORT CHASSISbind logical port LPORT to CHASSIS\n\
-  lport-unbind LPORT  reset the port binding of logical port LPORT\n\
+  lsp-bind PORT CHASSIS  bind logical port PORT to CHASSIS\n\
+  lsp-unbind PORTreset the port binding of logical port PORT\n\
 \n\
 Logical flow commands:\n\
   lflow-list [DATAPATH]   List logical flows for all or a single 
datapath\n\
@@ -602,7 +602,7 @@ cmd_chassis_del(struct ctl_context *ctx)
 }
 
 static void
-cmd_lport_bind(struct ctl_context *ctx)
+cmd_lsp_bind(struct ctl_context *ctx)
 {
 struct sbctl_context *sbctl_ctx = sbctl_context_cast(ctx);
 bool may_exist = shash_find(>options, "--may-exist") != NULL;
@@ -631,7 +631,7 @@ cmd_lport_bind(struct ctl_context *ctx)
 }
 
 static void
-cmd_lport_unbind(struct ctl_context *ctx)
+cmd_lsp_unbind(struct ctl_context *ctx)
 {
 struct sbctl_context *sbctl_ctx = sbctl_context_cast(ctx);
 bool must_exist = !shash_find(>options, "--if-exists");
@@ -1022,9 +1022,9 @@ static const struct ctl_command_syntax sbctl_commands[] = 
{
  "--if-exists", RW},
 
 /* Port binding commands. */
-{"lport-bind", 2, 2, "LPORT CHASSIS", pre_get_info, cmd_lport_bind, NULL,
+{"lsp-bind", 2, 2, "PORT CHASSIS", pre_get_info, cmd_lsp_bind, NULL,
  "--may-exist", RW},
-{"lport-unbind", 1, 1, "LPORT", pre_get_info, cmd_lport_unbind, NULL,
+{"lsp-unbind", 1, 1, "PORT", pre_get_info, cmd_lsp_unbind, NULL,
  "--if-exists", RW},
 
 /* Logical flow commands */
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index 4bbda73..c296f0e 100644
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -341,7 +341,7 @@ OVN_CONTROLLER_VTEP_START
 AT_CHECK([ovn-nbctl lsp-add br-test vif0])
 AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:02])
 AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
-AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
+AT_CHECK([ovn-sbctl lsp-bind vif0 ch0])
 
 # creates the logical switch in vtep and adds the corresponding logical
 # port to 'br-test'.
@@ -355,7 +355,7 @@ AT_CHECK([ovn-nbctl ls-add br-void])
 AT_CHECK([ovn-nbctl lsp-add br-void vif1])
 AT_CHECK([ovn-nbctl lsp-set-addresses vif1 f0:ab:cd:ef:01:02])
 AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
-AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
+AT_CHECK([ovn-sbctl lsp-bind vif1 ch1])
 OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
 
 # checks Ucast_Macs_Remote creation.
@@ -412,12 +412,12 @@ OVN_CONTROLLER_VTEP_START
 AT_CHECK([ovn-nbctl lsp-add br-test vif0])
 AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:02])
 AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
-AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
+AT_CHECK([ovn-sbctl lsp-bind vif0 ch0])
 
 # creates another vif in the same logical switch with duplicate mac.
 AT_CHECK([ovn-nbctl lsp-add br-test vif1])
 AT_CHECK([ovn-nbctl lsp-set-addresses vif1 f0:ab:cd:ef:01:02])
-AT_CHECK([ovn-sbctl lport-bind vif1 ch0])
+AT_CHECK([ovn-sbctl lsp-bind vif1 ch0])
 
 # creates the logical switch in vtep and adds the corresponding logical
 # port to 'br-test'.
@@ -446,7 +446,7 @@ AT_CHECK([ovn-nbctl ls-add br-void])
 AT_CHECK([ovn-nbctl lsp-add br-void vif1])
 AT_CHECK([ovn-nbctl lsp-set-addresses vif1 f0:ab:cd:ef:01:02])
 AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
-AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
+AT_CHECK([ovn-sbctl lsp-bind vif1 ch1])
 OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
 
 # creates another logical switch in vtep and adds the corresponding logical
diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
index 5232933..3b61d9d 100644
--- a/tests/ovn-sbctl.at
+++ b/tests/ovn-sbctl.at
@@ -83,7 +83,7 @@ AT_CHECK([ovn-nbctl ls-add br-test])
 AT_CHECK([ovn-nbctl lsp-add br-test vif0])
 AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:02])
 AT_CHECK([ovn-sbctl chassis-add ch0 stt 1.2.3.5])
-AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
+AT_CHECK([ovn-sbctl lsp-bind vif0 ch0])
 
 AT_CHECK([ovn-sbctl show], [0], [dnl
 Chassis "ch0"
@@ -95,7 +95,7 @@ Chassis "ch0"
 # adds another 'vif1'
 AT_CHECK([ovn-nbctl lsp-add br-test vif1])
 AT_CHECK([ovn-nbctl