[ovs-dev] Message could not be delivered

[youtubeunblocker]

The original message was received at Thu, 1 Sep 2016 11:27:04 +0530
from [5.6.209.9]

- The following addresses had permanent fatal errors -


- Transcript of the session follows -
... while talking to server 119.183.12.52:
>>> RCPT To:
<<< 550 5.1.1 ... User unknown

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

Re: [ovs-dev] one issue about vhost xstats with/without CRC

[Yang, Zhiyong]

Hi, Jesse:
Thanks a lot for your clear reply. Physical NIC  maybe conforms to
RFC2819 as follows:

etherStatsPkts64Octets OBJECT-TYPE
SYNTAX Counter32
UNITS "Packets"
MAX-ACCESS read-only
STATUS current
DESCRIPTION
"The total number of packets (including bad
packets) received that were 64 octets in length
(excluding framing bits but including FCS octets)."

The definition of prc64 from  Intel® 82599 10 GbE Controller
Datasheet is as following.
Number of good packets received that are 64 bytes in length (from  through , inclusively).
This registers counts packets that pass L2 filtering regardless on receive
enablement and does not include received flow control packets.

Thanks
Zhiyong Yang

> -Original Message-
> From: Jesse Gross [mailto:je...@kernel.org]
> Sent: Thursday, September 1, 2016 12:41 AM
> To: Yang, Zhiyong 
> Cc: d...@openvswitch.com
> Subject: Re: [ovs-dev] one issue about vhost xstats with/without CRC
> 
> On Wed, Aug 31, 2016 at 2:30 AM, Yang, Zhiyong 
> wrote:
> > Hi, all:
> >
> > Physical NIC has a set of hardware counters, such as
> > u64 prc64;
> > u64 prc127;
> > u64 prc255; etc.
> > DPDK counts the prc64 in two ways. Physical NIC counts prc64 with CRC
> > by hardware. Virtio computes the counter like prc64 without CRC. This
> > will cause the conflict, when a 64 packet from outer network is sent
> > to VM(virtio), NIC will show prc64 + 1, virtio will actually receive
> > the 64-4(CRC) = 60 bytes pkt,
> > undersize(<64) counter will be increased, since the Length of the
> > packet will be subtracted CRC by hardware(NIC offload enable) or
> > software((NIC offload disable)).
> >
> > if vhost xstats implements like NIC, It will solve the  consistency
> > issue. But when running vm to vm or virtio/vhost loopback test, xstats
> > counters will count non-existent CRC bytes .
> >
> > How should Packets length of virtio/vhost be counted from OVS
> perspective?
> > With or without CRC?
> 
> All other stats in OVS (including flows and other counters that don't come
> from hardware) count bytes without the CRC. Presumably it would be best to
> adjust the physical NIC stats with DPDK to do the same.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v3 2/5] ovn: Introduce l3 gateway router.

[Shi Xin Ruan]

Hi Guru,


I did not find the networking-ovn plugin for l3 gateway router. 
Is it still under development? 
Thanks.


Best Regards
Steve Ruan(阮诗新)

Tel: (86-021)60928749, 
Address: 5/F, Building 10, 399 Ke Yuan Road, Zhangjiang High-Tech Park, 
Shanghai 201203, PRC
上海浦东新区张江高科园区科苑路399号10号楼6楼




From:   Gurucharan Shetty 
To: dev@openvswitch.org
Date:   05/20/2016 01:59 PM
Subject:[ovs-dev] [PATCH v3 2/5] ovn: Introduce l3 gateway router.
Sent by:"dev" 



Currently OVN has distributed switches and routers. When a packet
exits a container or a VM, the entire lifecycle of the packet
through multiple switches and routers are calculated in source
chassis itself. When the destination endpoint resides on a different
chassis, the packet is sent to the other chassis and it only goes
through the egress pipeline of that chassis once and eventually to
the real destination.

When the packet returns back, the same thing happens. The return
packet leaves the VM/container on the chassis where it resides.
The packet goes through all the switches and routers in the logical
pipleline on that chassis and then sent to the eventual destination
over the tunnel.

The above makes the logical pipeline very flexible and easy. But,
creates a problem for cases where you need to add stateful services
(via conntrack) on switches and routers.

For l3 gateways, we plan to leverage DNAT and SNAT functionality
and we want to apply DNAT and SNAT rules on a router. So we ideally need
the packet to go through that router in both directions in the same
chassis. To achieve this, this commit introduces a new gateway router 
which is
static and can be connected to your distributed router via a switch.

To make minimal changes in OVN's logical pipeline, this commit
tries to make the switch port connected to a l3 gateway router look like
a container/VM endpoint for every other chassis except the chassis
on which the l3 gateway router resides. On the chassis where the
gateway router resides, the connection looks just like a patch port.

This is achieved by the doing the following:
Introduces a new type of port_binding record called 'gateway'.
On the chassis where the gateway router resides, this port behaves just
like the port of type 'patch'. The ovn-controller on that chassis
populates the "chassis" column for this record as an indication for
other ovn-controllers of its physical location. Other ovn-controllers
treat this port as they would treat a VM/Container port on a different
chassis.

Signed-off-by: Gurucharan Shetty 
---
 ovn/controller/binding.c|   3 +-
 ovn/controller/ovn-controller.c |   5 +-
 ovn/controller/patch.c  |  29 ++-
 ovn/controller/patch.h  |   3 +-
 ovn/northd/ovn-northd.c |  42 +++--
 ovn/ovn-nb.ovsschema|   9 +-
 ovn/ovn-nb.xml  |  15 
 ovn/ovn-sb.xml  |  35 +++-
 tests/ovn.at| 184 

 9 files changed, 309 insertions(+), 16 deletions(-)

diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index a0d8b96..e5e55b1 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -200,7 +200,8 @@ binding_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 }
 sbrec_port_binding_set_chassis(binding_rec, chassis_rec);
 }
-} else if (chassis_rec && binding_rec->chassis == chassis_rec) {
+} else if (chassis_rec && binding_rec->chassis == chassis_rec
+   && strcmp(binding_rec->type, "gateway")) {
 if (ctx->ovnsb_idl_txn) {
 VLOG_INFO("Releasing lport %s from this chassis.",
   binding_rec->logical_port);
diff --git a/ovn/controller/ovn-controller.c 
b/ovn/controller/ovn-controller.c
index 511b184..bc4c24f 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -364,8 +364,9 @@ main(int argc, char *argv[])
 _datapaths);
 }
 
-if (br_int) {
-patch_run(, br_int, _datapaths, 
_datapaths);
+if (br_int && chassis_id) {
+patch_run(, br_int, chassis_id, _datapaths,
+  _datapaths);
 
 struct lport_index lports;
 struct mcgroup_index mcgroups;
diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c
index 4808146..e8abe30 100644
--- a/ovn/controller/patch.c
+++ b/ovn/controller/patch.c
@@ -267,12 +267,28 @@ add_patched_datapath(struct hmap *patched_datapaths,
 static void
 add_logical_patch_ports(struct controller_ctx *ctx,
 const struct ovsrec_bridge *br_int,
+const char *local_chassis_id,
 struct shash *existing_ports,
 struct hmap *patched_datapaths)
 {
+const struct sbrec_chassis *chassis_rec;
+chassis_rec = 

[ovs-dev] db

[Umangkumar Patel]

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

[ovs-dev] [patch v1] ovn: ovn-ctl support to start ovn db servers in backup mode

[bschanmu]

From: Babu Shanmugam 

This patch adds support to start_ovsdb() function in ovn-ctl to start the
ovn db servers in backup mode. This can be done in the following ways
1. Use parameters --ovn-nb-sync-from-addr and --ovn-sb-sync-from-addr to
   set the addresses of the active server.
2. Create files $etcdir/ovnnb-active.conf and $etcdir/ovnsb-active.conf
   with the tcp url of the active servers.

If --ovn-nb-sync-from-addr and --ovn-sb-sync-from-addr is used, it will
overwrite the contents in the $etcdir/*.conf and use that server as the
active server.

Additional functions to promote a backup server to active and demote
active server to backup mode are also added in this patch

Signed-off-by: Babu Shanmugam 
---
 ovn/utilities/ovn-ctl | 75 +++
 1 file changed, 75 insertions(+)

diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index a4a9817..54c2a72 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -26,6 +26,8 @@ for dir in "$sbindir" "$bindir" /sbin /bin /usr/sbin 
/usr/bin; do
 done
 
 
+ovnnb_active_conf_file="$etcdir/ovnnb-active.conf"
+ovnsb_active_conf_file="$etcdir/ovnsb-active.conf"
 ## - ##
 ## start ##
 ## - ##
@@ -45,6 +47,42 @@ stop_ovsdb () {
 fi
 }
 
+promote_ovnnb() {
+if test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
+fi
+
+if test -e $ovnnb_active_conf_file; then
+ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnnb_active_conf_file`
+else
+echo >&2 "$0: active server details not set"
+exit 1
+fi
+}
+
+promote_ovnsb() {
+if test ! -z "$DB_SB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_SB_SYNC_FROM_ADDR:$DB_SB_SYNC_FROM_PORT" > 
$ovnsb_active_conf_file
+fi
+
+if test -e $ovnsb_active_conf_file; then
+ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/set-active-ovsdb-server `cat $ovnsb_active_conf_file`
+else
+echo >&2 "$0: active server details not set"
+exit 1
+fi
+}
+
+demote_ovnnb() {
+rm -f $ovnnb_active_conf_file
+ovs-appctl -t $rundir/ovnnb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
+}
+
+demote_ovnsb() {
+rm -f $ovnsb_active_conf_file
+ovs-appctl -t $rundir/ovnsb_db.ctl 
ovsdb-server/disconnect-active-ovsdb-server
+}
+
 start_ovsdb () {
 # Check and eventually start ovsdb-server for Northbound DB
 if ! pidfile_is_running $DB_NB_PID; then
@@ -54,6 +92,14 @@ start_ovsdb () {
 
 set "$@" --detach $OVN_NB_LOG --log-file=$OVN_NB_LOGFILE 
--remote=punix:$DB_NB_SOCK --remote=ptcp:$DB_NB_PORT:$DB_NB_ADDR 
--pidfile=$DB_NB_PID --unixctl=ovnnb_db.ctl
 
+if test ! -z "$DB_NB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_NB_SYNC_FROM_ADDR:$DB_NB_SYNC_FROM_PORT" > 
$ovnnb_active_conf_file
+fi
+
+if test -e $ovnnb_active_conf_file; then
+set "$@" --sync-from=`cat $ovnnb_active_conf_file`
+fi
+
 $@ $DB_NB_FILE
 fi
 
@@ -64,6 +110,15 @@ start_ovsdb () {
 set ovsdb-server
 
 set "$@" --detach $OVN_SB_LOG --log-file=$OVN_SB_LOGFILE 
--remote=punix:$DB_SB_SOCK --remote=ptcp:$DB_SB_PORT:$DB_SB_ADDR 
--pidfile=$DB_SB_PID --unixctl=ovnsb_db.ctl
+
+if test ! -z "$DB_SB_SYNC_FROM_ADDR"; then
+echo "tcp:$DB_SB_SYNC_FROM_ADDR:$DB_SB_SYNC_FROM_PORT" > 
$ovnsb_active_conf_file
+fi
+
+if test -e $ovnsb_active_conf_file; then
+set "$@" --sync-from=`cat $ovnsb_active_conf_file`
+fi
+
 $@ $DB_SB_FILE
 fi
 }
@@ -176,12 +231,16 @@ set_defaults () {
 DB_NB_FILE=$dbdir/ovnnb_db.db
 DB_NB_ADDR=0.0.0.0
 DB_NB_PORT=6641
+DB_NB_SYNC_FROM_ADDR=
+DB_NB_SYNC_FROM_PORT=6641
 
 DB_SB_SOCK=$rundir/ovnsb_db.sock
 DB_SB_PID=$rundir/ovnsb_db.pid
 DB_SB_FILE=$dbdir/ovnsb_db.db
 DB_SB_ADDR=0.0.0.0
 DB_SB_PORT=6642
+DB_SB_SYNC_FROM_ADDR=
+DB_SB_SYNC_FROM_PORT=6642
 
 DB_NB_SCHEMA=$datadir/ovn-nb.ovsschema
 DB_SB_SCHEMA=$datadir/ovn-sb.ovsschema
@@ -272,6 +331,10 @@ File location options:
   --db-sb-port=PORTOVN Southbound db ptcp port (default: $DB_SB_PORT)
   --ovn-nb-logfile=FILE OVN Northbound log file (default: $OVN_NB_LOGFILE)
   --ovn-sb-logfile=FILE OVN Southbound log file (default: $OVN_SB_LOGFILE)
+  --db-nb-sync-from-addr=ADDR OVN Northbound active db tcp address (default: 
$DB_NB_SYNC_FROM_ADDR)
+  --db-nb-sync-from-port=PORT OVN Northdbound active db tcp port (default: 
$DB_NB_SYNC_FROM_PORT)
+  --db-sb-sync-from-addr=ADDR OVN Southbound active db tcp address (default: 
$DB_SB_SYNC_FROM_ADDR)
+  --db-sb-sync-from-port=ADDR OVN Southbound active db tcp port (default: 
$DB_SB_SYNC_FROM_PORT)
 
 Default directories with "configure" option and environment variable override:
   logs: /usr/local/var/log/openvswitch (--with-logdir, OVS_LOGDIR)
@@ -377,6 +440,18 @@ 

Re: [ovs-dev] [PATCH 2/2] ovn-controller: Drop incremental processing from encapsulation code.

[Ben Pfaff]

On Wed, Aug 31, 2016 at 07:33:06PM -0400, Russell Bryant wrote:
> On Wed, Aug 31, 2016 at 5:25 PM, Ben Pfaff  wrote:
> 
> > This commit reverts encaps.c to its content just before commit 1d45d5a9666d
> > (ovn-controller: Change encaps_run to work incrementally.).  I then
> > reintroduced the UDP checksum support originallly added in commit
> > 36283d7884f3 (ovn-controller: Use UDP checksums when creating Geneve
> > tunnels.)  I also read the other commits following the incremental
> > processing commit to verify that this change didn't lose any bug fixes.
> >
> > This commit takes advantage of the "addvalue" and "delvalue" functions
> > now available in the IDL to simplify some code.
> >
> > Signed-off-by: Ben Pfaff 
> >
> 
> Acked-by: Russell Bryant 

Thanks for the reviews, Russell.  I applied these two patches to master
and branch-2.6.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn: log dhcp responses at INFO for debugging

[Ben Pfaff]

On Thu, Sep 01, 2016 at 12:20:46AM +, Ramu Ramamurthy wrote:
> Since dhcp responses are important for debugging the vif lifecycle,
> we want to log them at info level which is the default log level.
> A logsearch (on macaddr) can quickly tell the dhcp status using 
> such messages. There is no need to rate limit such logs
> because, we expect dhcp messages to be at low rate normally.

Any log message that can be generated by receiving a network packet
needs to be rate-limited.  If the messages are actually received at a
low rate, then there's no disadvantage.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofproto: Do not override internal port MTU.

[Darrell Ball]

On Wed, Aug 31, 2016 at 6:48 PM, Joe Stringer  wrote:

> On 31 August 2016 at 14:52, Daniele Di Proietto 
> wrote:
> > Open vSwitch controls the MTU of internal ports and sets it to the
> > minimum of physical ports MTU on the same bridge.
> >
> > Commit 47bf118665a3("ofproto: Always set MTU for new internal ports.")
> > made this more consistent.  Now the MTU is always set, even when the
> > bridge doesn't have any physical ports (e.g. when it has only an
> > internal device and a tunnel).
> >
> > This latest change broke the system testsuite.  Some tests need to
> > lower internal devices MTU because they use tunnels and they want to
> > work with a 1500 bytes underlay.
> >
> > There are other valid usecases where the user/controller needs to
> > configure the internal devices MTU (like mpls push actions, double vlans
> > or any overlay) and there's no way for Open vSwitch to know what the
> > appropriate MTU should be.
> >
> > Since in the general case Open vSwitch is not able to configure a
> > reasonable MTU for internal devices, this commit removes the feature
> > entirely.
> >
> > Now the user/controller is entirely responsible for configuring the MTU
> > of internal ports.  Other hybrid solutions are possible (like not
> > touching the internal interfaces MTU, unless there's a physical device),
> > but they make the current MTU dependent on the previous state of the
> > system (if there was at some point a physical device the MTU would be
> > touched, but it wouldn't be possible to restore it).
> >
> > This change breaks compatibility with previous versions on Open vSwitch.
> >
> > Signed-off-by: Daniele Di Proietto 
>
> So I guess that prior to this patch, there is some sort of contract
> that the MTU of the internal interfaces are entirely managed by OVS,
> and it's set to the minimum of all non-internal devices, then the MTU
> of all other devices is managed by the user/controller. The trouble
> with this is cases where, for instance, the user wants to attach the
> local networking stack to some kind of overlay (tunnel, mpls,
> QinQ,...) and the user either cannot, or does not want to, change the
> underlay MTU to allow encapsulating full 1500B frames. In such a case,
> including the OVS testsuite today (which admittedly can be taught to
> be smarter), the current behaviour prohibits the user from providing
> that more detailed information for the bridge interface. Maybe it
> works in /some/ cases, but OVS will override the MTU if any sort of
> configuration is changed on that bridge.
>

ip and mpls tunnels that traverse multiple transport nodes have another
issue;
PMTU could help in many cases, but not all.
Even just local to the source transport node, figuring out the path taken,
which
can change requires added infra to get it right and optimal in an
automatic, general
and timely manner. I am not sure the added complexity is worth it,
in the case of ovs.

If OVS is in the business of controlling non-internal interface MTUs, as it
is with the addition of the mtu_request column, then it seems useful to be
able
to also "manually" control the mtu of internal interfaces, when needed, as
this
patch proposes.



>
> On the flip side, if a user wants to, for instance, configure OVS to
> provide switching from a physical NIC that supports jumbo frames, then
> today they simply set the MTU of the physical device and OVS takes
> care of the internal device. With this patch, the user/controller
> would be responsible for changing the internal device MTU as well.
>
> One thought I have is that it seems like maybe the cases that OVS is
> implicitly figuring out the MTU today are cases where the user already
> has to figure out MTU for another device, so maybe forcing them to
> also configure the internal port's MTU is not a huge burden. Is this
> the case?
>

Dealing with internal interface MTU configuration seems like some added
burden, including users understanding the internal interfaces relationships.
I thought that the main benefit of the present auto-configuration
of MTU for internal interfaces is simplicity.

I am not sure completely removing the default behavior of auto-configuring
internal interface MTUs to the minimum of non-internal interfaces is the
way to go, even though it is not optimal in all cases; it seems like there
might be some simplicity benefit for some users.
Being able to override this default behavior, as proposed with this patch
seems useful.



>
> The other thought I have is that maybe users are more familiar with
> doing "ip link set dev breth0 mtu 1600" rather than using OVSDB to set
> up the MTU. I suspect that even if this is the case, it's simpler if
> OVS doesn't have any implicit logic to reconfigure the bridge device
> MTU rather than potentially override the user configuration. If the
> user wants to do things this way, they can, but it just won't persist.
> If they want it to persist, they can set the 

[ovs-dev] [v3] ovsdb: Replication usability improvements

[Andy Zhou]

Added the '--no-sync' option base on feedbacks of current
implementation.

Added appctl command "ovsdb-server/sync-status" based on feedbacks
of current implementation.

Added the RPL_S_INIT state in the state machine. This state is
not strictly necessary for the replication state machine, but is
introduced to make sure the state is update immediately when
the state machine is reset, via replication_init(). Without it
ovsdb/sync-status may display "replicating" or crash, if the command
is issued between after replication_init() is called, but before
the state variable is updated from replication_run().

Added a test to simulate the integration of HA manager with OVSDB
server using replication.

Other documentation and API improvements.

Tested-by: Numan Siddique 
Signed-off-by: Andy Zhou 
---
 Documentation/OVSDB-replication.md |   2 +-
 ovsdb/jsonrpc-server.c |   6 +
 ovsdb/jsonrpc-server.h |   1 +
 ovsdb/ovsdb-server.1.in|  24 ++--
 ovsdb/ovsdb-server.c   | 251 ++---
 ovsdb/replication.c|  89 ++---
 ovsdb/replication.h|  28 -
 ovsdb/replication.man  |  15 ++-
 tests/ovsdb-server.at  | 105 ++--
 9 files changed, 408 insertions(+), 113 deletions(-)

diff --git a/Documentation/OVSDB-replication.md 
b/Documentation/OVSDB-replication.md
index 74c9500..a9ab5cc 100644
--- a/Documentation/OVSDB-replication.md
+++ b/Documentation/OVSDB-replication.md
@@ -144,7 +144,7 @@ commands are the following.
 between the active server and frees the memory used for the replication
 configuration.
 
--   ovsdb-server/set-sync-excluded-tables {db:table,...}: sets the tables list
+-   ovsdb-server/set-sync-exclude-tables {db:table,...}: sets the tables list
 that will be excluded from being replicated.
 
 -   ovsdb-server/get-sync-excluded-tables: gets the tables list that is
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 45975a3..427026d 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -343,6 +343,12 @@ ovsdb_jsonrpc_server_reconnect(struct ovsdb_jsonrpc_server 
*svr, bool read_only)
 }
 }
 
+bool
+ovsdb_jsonrpc_server_is_read_only(struct ovsdb_jsonrpc_server *svr)
+{
+return svr->read_only;
+}
+
 void
 ovsdb_jsonrpc_server_run(struct ovsdb_jsonrpc_server *svr)
 {
diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h
index 955bbe4..f04b1e9 100644
--- a/ovsdb/jsonrpc-server.h
+++ b/ovsdb/jsonrpc-server.h
@@ -65,6 +65,7 @@ void ovsdb_jsonrpc_server_reconnect(struct 
ovsdb_jsonrpc_server *, bool read_onl
 
 void ovsdb_jsonrpc_server_run(struct ovsdb_jsonrpc_server *);
 void ovsdb_jsonrpc_server_wait(struct ovsdb_jsonrpc_server *);
+bool ovsdb_jsonrpc_server_is_read_only(struct ovsdb_jsonrpc_server *);
 
 void ovsdb_jsonrpc_server_get_memory_usage(const struct ovsdb_jsonrpc_server *,
struct simap *usage);
diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index d1ba83b..e2e96ae 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -36,7 +36,7 @@ If none is specified, the default is \fB@DBDIR@/conf.db\fR.  
The database
 files must already have been created and initialized using, for
 example, \fBovsdb\-tool create\fR.
 .
-.SH "ACTIVE and BACKUP "
+.SH "ACTIVE and BACKUP"
 \fBovsdb\-server\fR runs either as a backup server, or as an active server.
 When  \fBovsdb\-server\fR is running as a backup server, all transactions that
 can modify the database content, including the lock commands are rejected.
@@ -45,12 +45,12 @@ When \fBovsdb\-server\fR role changes, all existing client 
connection are
 reset, requiring clients to reconnect to the server.
 .PP
 By default, \fBovsdb\-server\fR runs as an active server, except when the
-\fB\-\-sync\-from=\fIserver\fR command line option is specified.  During
-runtime, \fBovsdb\-server\fR role can be switch by using appctl commands.
+\fB\-\-sync\-from=\fIserver\fR command line option is specified and without
+the \fB\-\-no\-sync option\fR.  During runtime, \fBovsdb\-server\fR role can 
be switch by using appctl commands.
 .PP
 \fBovsdb-server/connect\-active\-ovsdb\-server\fR switches
-\fBovsdb\-server\fR role into a backup server, Conversely,
-\fBovsdb-server/disconnect\-active\-ovsdb\-server\fR changes server into
+\fBovsdb\-server\fR into a backup server, Conversely,
+\fBovsdb-server/disconnect\-active\-ovsdb\-server\fR switches server into
 an active one.
 .
 .SH OPTIONS
@@ -220,12 +220,22 @@ specified by 
\fBovsdb\-server/set\-active\-ovsdb\-server\fR.
 .IP "\fBovsdb\-server/disconnect\-active\-ovsdb\-server"
 Causes \fBovsdb\-server\fR to  stop  synchronizing  its  databases with a 
active server.
 .
-.IP "\fBovsdb\-server/set\-sync\-excluded\-tables 
\fIdb\fB:\fItable\fR[\fB,\fIdb\fB:\fItable\fR]..."
+.IP "\fBovsdb\-server/set\-sync\-exclude\-tables 

Re: [ovs-dev] [replication SMv2 6/7] OVSDB: Reimplement replication. Using a state machine.

[Andy Zhou]

On Tue, Aug 30, 2016 at 2:35 PM, Ben Pfaff  wrote:

> On Fri, Aug 26, 2016 at 04:15:53PM -0700, Andy Zhou wrote:
> > Current replication uses blocking transactions, which are error prone
> > in practice, especially in handling RPC connection flapping to the
> > active server.
> >
> > Signed-off-by: Andy Zhou 
>
> Much better.  Thank you.
>
> Acked-by: Ben Pfaff 
>
>
> Ben, thank you so much for review and suggestions. I have folded them all
pushed patch 1-6
to master.  I will repost patch 7 as v3.  I will back port all 7 patches to
branch 2.6 once patch 7
is reviewed and acked.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofproto: Do not override internal port MTU.

[Joe Stringer]

On 31 August 2016 at 14:52, Daniele Di Proietto  wrote:
> Open vSwitch controls the MTU of internal ports and sets it to the
> minimum of physical ports MTU on the same bridge.
>
> Commit 47bf118665a3("ofproto: Always set MTU for new internal ports.")
> made this more consistent.  Now the MTU is always set, even when the
> bridge doesn't have any physical ports (e.g. when it has only an
> internal device and a tunnel).
>
> This latest change broke the system testsuite.  Some tests need to
> lower internal devices MTU because they use tunnels and they want to
> work with a 1500 bytes underlay.
>
> There are other valid usecases where the user/controller needs to
> configure the internal devices MTU (like mpls push actions, double vlans
> or any overlay) and there's no way for Open vSwitch to know what the
> appropriate MTU should be.
>
> Since in the general case Open vSwitch is not able to configure a
> reasonable MTU for internal devices, this commit removes the feature
> entirely.
>
> Now the user/controller is entirely responsible for configuring the MTU
> of internal ports.  Other hybrid solutions are possible (like not
> touching the internal interfaces MTU, unless there's a physical device),
> but they make the current MTU dependent on the previous state of the
> system (if there was at some point a physical device the MTU would be
> touched, but it wouldn't be possible to restore it).
>
> This change breaks compatibility with previous versions on Open vSwitch.
>
> Signed-off-by: Daniele Di Proietto 

So I guess that prior to this patch, there is some sort of contract
that the MTU of the internal interfaces are entirely managed by OVS,
and it's set to the minimum of all non-internal devices, then the MTU
of all other devices is managed by the user/controller. The trouble
with this is cases where, for instance, the user wants to attach the
local networking stack to some kind of overlay (tunnel, mpls,
QinQ,...) and the user either cannot, or does not want to, change the
underlay MTU to allow encapsulating full 1500B frames. In such a case,
including the OVS testsuite today (which admittedly can be taught to
be smarter), the current behaviour prohibits the user from providing
that more detailed information for the bridge interface. Maybe it
works in /some/ cases, but OVS will override the MTU if any sort of
configuration is changed on that bridge.

On the flip side, if a user wants to, for instance, configure OVS to
provide switching from a physical NIC that supports jumbo frames, then
today they simply set the MTU of the physical device and OVS takes
care of the internal device. With this patch, the user/controller
would be responsible for changing the internal device MTU as well.

One thought I have is that it seems like maybe the cases that OVS is
implicitly figuring out the MTU today are cases where the user already
has to figure out MTU for another device, so maybe forcing them to
also configure the internal port's MTU is not a huge burden. Is this
the case?

The other thought I have is that maybe users are more familiar with
doing "ip link set dev breth0 mtu 1600" rather than using OVSDB to set
up the MTU. I suspect that even if this is the case, it's simpler if
OVS doesn't have any implicit logic to reconfigure the bridge device
MTU rather than potentially override the user configuration. If the
user wants to do things this way, they can, but it just won't persist.
If they want it to persist, they can set the configuration in OVSDB.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 1/5] ofproto-dpif-xlate: Don't unwildcard tunnel attributes on set.

[Daniele Di Proietto]

On 31/08/2016 11:32, "Jarno Rajahalme"  wrote:

>I’d put the registers and metadata field to the ‘false’ and also maybe 
>non-writeable fields (ether type, frags, nw_proto, etc.) in to 
>OVS_NOT_REACHED() case, just in case.
>
>  Jarno

Agreed, done

Thanks,

Daniele

>
>> On Aug 31, 2016, at 10:38 AM, Jesse Gross  wrote:
>> 
>> On Tue, Aug 30, 2016 at 6:47 PM, Daniele Di Proietto
>>  wrote:
>>> When translating a set action we also unwildcard the field in question.
>>> This is done to correctly translate set actions with the value identical
>>> to the ingress flow, like in the following example:
>>> 
>>> flow table:
>>> 
>>> tcp,actions=set_field:80->tcp_dst,output:5
>>> 
>>> ingress packet:
>>> 
>>> ...,tcp,tcp_dst=80
>>> 
>>> datapath flow
>>> 
>>> ...,tcp(dst=80) actions:5
>>> 
>>> The datapath flow must exact match the target field, because the actions
>>> do not include a set field. (Otherwise a packet coming in with a
>>> different tcp_dst would be matched, and its port wouldn't be altered).
>>> 
>>> Tunnel attributes behave differently: at the datapath layer, before
>>> action processing they're cleared (we do the same at the ofproto layer
>>> in xlate_actions()).  Therefore there's no need to unwildcard them,
>>> because a set action would always be detected (since we zero them at the
>>> beginning of xlate_ations()).
>>> 
>>> This fixes a problem related to the handling of Geneve options.
>>> Unwildcarding non existing Geneve options (as done by a
>>> set_field:X->tun_metadata on a packet coming from a non-tunnel
>>> interface) would be problematic for the datapaths: the ODP translation
>>> functions cannot express a match on non existing Geneve options (unlike
>>> on other attributes), and the userspace datapath wouldn't be able to
>>> translate them to "userspace datapath format".  In both cases the
>>> installed flow would be deleted by revalidation at the first
>>> opportunity.
>>> 
>>> Signed-off-by: Daniele Di Proietto 
>> 
>> I think there might be some more obscure ways of triggering this
>> problem that still exist. Basically anything that can use a register
>> as a target is a potential issue. For example, stack_pop, bundle, and
>> multipath all look like they have the same masking behavior.
>> 
>> I do have a general solution in this patch (look at the bottom of
>> xlate_actions() where it is adjusting the wildcards):
>> http://openvswitch.org/pipermail/dev/2016-August/078484.html
>> 
>> I didn't realize that it was addressing an existing issue though and
>> that patch certainly isn't suitable for anything other than master.
>> 
>> I'm also not really a big fan of the way I handled it there since it's
>> a pretty coarse way to do it. I would be happy to drop it if we can
>> feel comfortable that we got all of the callsites (now and in the
>> future) using your method. Perhaps we can just create a helper
>> function with this check builtin and then use it everywhere? That
>> might be enough to be confident about the future.
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 1/5] ofproto-dpif-xlate: Don't unwildcard tunnel attributes on set.

[Daniele Di Proietto]

On 31/08/2016 10:38, "Jesse Gross"  wrote:

>On Tue, Aug 30, 2016 at 6:47 PM, Daniele Di Proietto
> wrote:
>> When translating a set action we also unwildcard the field in question.
>> This is done to correctly translate set actions with the value identical
>> to the ingress flow, like in the following example:
>>
>> flow table:
>>
>> tcp,actions=set_field:80->tcp_dst,output:5
>>
>> ingress packet:
>>
>> ...,tcp,tcp_dst=80
>>
>> datapath flow
>>
>> ...,tcp(dst=80) actions:5
>>
>> The datapath flow must exact match the target field, because the actions
>> do not include a set field. (Otherwise a packet coming in with a
>> different tcp_dst would be matched, and its port wouldn't be altered).
>>
>> Tunnel attributes behave differently: at the datapath layer, before
>> action processing they're cleared (we do the same at the ofproto layer
>> in xlate_actions()).  Therefore there's no need to unwildcard them,
>> because a set action would always be detected (since we zero them at the
>> beginning of xlate_ations()).
>>
>> This fixes a problem related to the handling of Geneve options.
>> Unwildcarding non existing Geneve options (as done by a
>> set_field:X->tun_metadata on a packet coming from a non-tunnel
>> interface) would be problematic for the datapaths: the ODP translation
>> functions cannot express a match on non existing Geneve options (unlike
>> on other attributes), and the userspace datapath wouldn't be able to
>> translate them to "userspace datapath format".  In both cases the
>> installed flow would be deleted by revalidation at the first
>> opportunity.
>>
>> Signed-off-by: Daniele Di Proietto 
>
>I think there might be some more obscure ways of triggering this
>problem that still exist. Basically anything that can use a register
>as a target is a potential issue. For example, stack_pop, bundle, and
>multipath all look like they have the same masking behavior.

You're right, thanks.  I added two extra checks (bundle and multipath
share part of the code).

>
>I do have a general solution in this patch (look at the bottom of
>xlate_actions() where it is adjusting the wildcards):
>http://openvswitch.org/pipermail/dev/2016-August/078484.html
>
>I didn't realize that it was addressing an existing issue though and
>that patch certainly isn't suitable for anything other than master.

Perhaps the commit message was too specific about Geneve options, because
that's how I found this originally, but the issue applies also to other
tunnel metadata (tunnel id, tunnel flags, ...)

>
>I'm also not really a big fan of the way I handled it there since it's
>a pretty coarse way to do it. I would be happy to drop it if we can
>feel comfortable that we got all of the callsites (now and in the
>future) using your method. Perhaps we can just create a helper
>function with this check builtin and then use it everywhere? That
>might be enough to be confident about the future.

I agree with you, I'd prefer to fix it on the set action, if possible.

Thanks for you input!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2] ovn-nbctl: Add LB commands.

[nickcooper-zhangtonghao]

Sorry for missing one of your points in the previous review.

I will submit v3 patch to support that function.

> On Aug 31, 2016, at 11:11 PM, Guru Shetty  wrote:
> 
> On 29 August 2016 at 20:12, nickcooper-zhangtonghao 
>  > wrote:
> Add a name column for the load balancer.  This must be unique among
> all load balancers.  This name has no special meaning or purpose
> other than to provide convenience for human interaction with the
> ovn-nb database.  This patch provides the command line to
> create load-balancer and add the unit tests.
> I think you missed one of my points in the previous review. I want to be able 
> to create the load balancer independently. 
>  i..e I don't want to wait till I add it to a switch. This helps in creating 
> a load-balancer and adding it to multiple switches. Also, I have code sent 
> out for review to add the load-balancer to the gateway router too. So the 
> commands should work with that too.
> 
> So with your patch, can I create a load balancer independently and add it to 
> multiple switches?

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

[ovs-dev] Z TJXFDGSYIKDBR

[jvu]

Å|ƒ0ˆ;«)3`Ïü¶¸^ÞÊ6Éuh›½‰Ïµó"VÖð
tÔÊn¨Õ¼DzÎßD5åPÃqÇjöòšÄs¶,ÎLlYl3Ĥº7UNóÖß`Ó¶&¢`8»0MåçZ>{H¦éjϏ›VAB–·á™hÚ¡DVlä´×ª'h)¡
…¨:'陋ø5‡3öâ8¶ ¯ÕÞÐc¶ªS>\ÉAêû[ú[\%·¨Ñé#Ãî 
‰œQSÕçwjp~Dvëãé~t%QîmHϏÌÞz%g{Šµy7Ý//u3éø·ñ•vàyF«¥Œ“
mvòK¶ÆVÇ878Š„ûj
úPS$ÔàwÕVø)Öpw~{6Ez¿
I
œ*n̉O#´f’Ä®ü¹{ëMÚþn–`>¬
z˜Mò¤™ò?Ò觑4zr~xÑ\˜º'3Bߟ"é# 
Lù¯i».¼¦ñvÒz¸g®#£ØÞ¹áõüón•Í¿¨j8ûjúQŒpfA7qLèwÕÝÜëåÙ
¬ŸÅ?-Ú'h뒬ëÇ®é—Y´5ʐ¨¦‚TLÄBÈĘ`Û¢ë©wl¼UJ•1p±´‚"V›r2n[‹›îuþwñþ·(ü—?üÍáíÔÈc–oý¨:"—×"1wŸœ
 #F“aÛÑðҌ yàð¦#ºvk¤mR)çšo½³à¬>¤zNìîÊ(nÐØþÃ.¢49kê[[ï#÷ó,±–:­j¿
™‘eI²‹ðãcÆyRj÷ݜpºgŒÀd¯§j&*™$üv¼ûޕ*~]½öQ½D¢7DñÏù§ÆCçrܟ4í|ªèi;s¦ì¥N;:ØWÎîâbºýc™£Ñ(?ð
ƒ>ˆøH%VL´Ž«SsÚx}^yØՈÄ”BMO’¾`ëÜɟ"ŒU¥4°Œf]W
Ooäí‰)‚ëaóq˜^‚Y3ÛÚ¬Ý>ù¹w˜’ä
ÇZ…ëË5wÉkv%¹3ºŒÆÕëš×B^bþXŽ%(J8}ªŸ#,‹ÖÉm͕áƒOñBf«zè ;
D{,¥øÒPǯê-µ·"´nè\!|4ÞÕ<¯Ë¼n½ª½Æ§1«<†qñCF üðóÒÀ±êæU¶&‹ ¨á ýª¦ÝƒyÎBùÆ«…
lj£ŸA,µnvãÌÔ,gÖB_±¬ÓèbÁ‡Ôóe«ÐBé>vžkº´Ôš¼òŸìP8z'ßõ5v²Gß;âÀÚ9*s‘ÆÐÃGØóQ 
ûîR¶ƒï3n[èlƒÞ‹~f)Û-¢}vòÜòõ4
tÝsfŠ•GìÙÍ_âf¼¬º*-R¹–woûÍ敠’w—îpXž8:ìh3¿ý\í…ayù˜À½wÞÞÙñ­½êsf»2JÌ·–Š‡
—!Š£ÂE-˜ê|Q4¢áX×wÜДµ‘è;¼ôÞÇ¡Èß½¨jâ<8 *ñ·^îõ/¾ášbô ¬^š‚˜1”¥á`²ÐåatcþÉ4QÖljd:‡ 
ˆ"þõ×EÜSVOŠOá“
FÖ ¡©ó(ÕÀݤUùp}*ž»{i¤
XŠ¤4­3²×¢£.#-÷~¡Æ®ž3ԉ$ì䁄pÖÏPOÈnUïmAÎ}ýé%3ì¥ê´uxo4Č[K%ý«n3®]tPG²·„F¨žI¾aò—ß{·ia
…ÍOBæÆ`¾ö
¼d›¤vÏûMH

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

[ovs-dev] Error

[news]

This message was not delivered due to the following reason:

Your message was not delivered because the destination server was
not reachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message could not be delivered within 7 days:
Host 112.28.177.182 is not responding.

The following recipients did not receive this message:


Please reply to postmas...@openvswitch.org
if you feel this message to be in error.

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

[ovs-dev] [PATCH] ovn: log dhcp responses at INFO for debugging

[Ramu Ramamurthy]

Since dhcp responses are important for debugging the vif lifecycle,
we want to log them at info level which is the default log level.
A logsearch (on macaddr) can quickly tell the dhcp status using 
such messages. There is no need to rate limit such logs
because, we expect dhcp messages to be at low rate normally.

Logs appear like this:

2016-09-01T00:08:16Z|00014|pinctrl|INFO|DHCPOFFER fa:16:3e:25:b0:78 10.0.0.5
2016-09-01T00:08:16Z|00015|pinctrl|INFO|DHCPACK fa:16:3e:25:b0:78 10.0.0.5

Signed-off-by: Ramu Ramamurthy 
---
 ovn/controller/pinctrl.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index d65e213..ad42fe9 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -354,6 +354,12 @@ pinctrl_handle_put_dhcp_opts(
 pin->packet = dp_packet_data(_out);
 pin->packet_len = dp_packet_size(_out);
 
+/* Log the response. */
+const struct eth_header *l2 = dp_packet_l2(_out);
+VLOG_INFO("DHCP%s "ETH_ADDR_FMT" "IP_FMT"",
+ msg_type == DHCP_MSG_OFFER ? "OFFER" : "ACK",
+ ETH_ADDR_ARGS(l2->eth_src), IP_ARGS(*offer_ip));
+
 success = 1;
 exit:
 if (!ofperr) {
-- 
1.8.3.1

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

[ovs-dev] Voice Message from Outside Caller (1m 38s)

[Peach Telecom]

Voice Message Arrived on Friday, Aug 26 @ 5: 25 AM
Name: Outside Caller
Number: Unavailable
Duration: 1m 38s
_
OPENVSWITCH.COM SV9100 InMail
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Voice Message from Outside Caller (1m 34s)

[Peach Telecom]

Voice Message Arrived on Friday, Aug 26 @ 8: 25 AM
Name: Outside Caller
Number: Unavailable
Duration: 1m 34s
_
OPENVSWITCH.COM SV9100 InMail
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] datapath-windows: remove invalid ASSERT in Flow.c

[Yin Lin]

Hi Nithin,

Instead of removing the assertion, can you change it to:

ASSERT(!key->tunKey.dst || offset == OvsGetFlowL2Offset(>tunKey));

I fixed it for OvsLookupFlow but somehow overlooked OvsHashFlow in my
Geneve patch.

Best regards,
Yin Lin

On Wed, Aug 31, 2016 at 3:33 AM, Nithin Raju  wrote:

> Since the Geneve changes, the key->l2.offset will no longer be 0 when
> the tunnel key is valid within the OVS flow key. key->l2.offset would
> be determined by the amount of tunnel options.
>
> Signed-off-by: Nithin Raju 
> ---
>  datapath-windows/ovsext/DpInternal.h | 9 ++---
>  datapath-windows/ovsext/Flow.c   | 1 -
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/datapath-windows/ovsext/DpInternal.h
> b/datapath-windows/ovsext/DpInternal.h
> index 22599a0..f62fc55 100644
> --- a/datapath-windows/ovsext/DpInternal.h
> +++ b/datapath-windows/ovsext/DpInternal.h
> @@ -157,17 +157,20 @@ typedef union OvsIPv4TunnelKey {
>  uint64_t attr[NUM_PKT_ATTR_REQUIRED];
>  } OvsIPv4TunnelKey; /* Size of 280 byte. */
>
> -__inline uint8_t TunnelKeyGetOptionsOffset(const OvsIPv4TunnelKey *key)
> +static __inline uint8_t
> +TunnelKeyGetOptionsOffset(const OvsIPv4TunnelKey *key)
>  {
>  return TUN_OPT_MAX_LEN - key->tunOptLen;
>  }
>
> -__inline uint8_t* TunnelKeyGetOptions(OvsIPv4TunnelKey *key)
> +static __inline uint8_t *
> +TunnelKeyGetOptions(OvsIPv4TunnelKey *key)
>  {
>  return key->tunOpts + TunnelKeyGetOptionsOffset(key);
>  }
>
> -__inline uint16_t TunnelKeyGetRealSize(OvsIPv4TunnelKey *key)
> +static __inline uint16_t
> +TunnelKeyGetRealSize(OvsIPv4TunnelKey *key)
>  {
>  return sizeof(OvsIPv4TunnelKey) - TunnelKeyGetOptionsOffset(key);
>  }
> diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/
> Flow.c
> index 7a57f96..439fb28 100644
> --- a/datapath-windows/ovsext/Flow.c
> +++ b/datapath-windows/ovsext/Flow.c
> @@ -2595,7 +2595,6 @@ OvsHashFlow(const OvsFlowKey *key)
>  UINT8 *start;
>
>  ASSERT(key->tunKey.dst || offset == sizeof(OvsIPv4TunnelKey));
> -ASSERT(!key->tunKey.dst || offset == 0);
>  start = (UINT8 *)key + offset;
>  return OvsJhashBytes(start, size, 0);
>  }
> --
> 2.6.2
>
> ___
> 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] Voice Message from Outside Caller (2m 21s)

[Peach Telecom]

Voice Message Arrived on Friday, Aug 26 @ 5: 35 AM
Name: Outside Caller
Number: Unavailable
Duration: 2m 21s
_
OPENVSWITCH.COM SV9100 InMail
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] ovn-controller: Drop incremental processing from encapsulation code.

[Russell Bryant]

On Wed, Aug 31, 2016 at 5:25 PM, Ben Pfaff  wrote:

> This commit reverts encaps.c to its content just before commit 1d45d5a9666d
> (ovn-controller: Change encaps_run to work incrementally.).  I then
> reintroduced the UDP checksum support originallly added in commit
> 36283d7884f3 (ovn-controller: Use UDP checksums when creating Geneve
> tunnels.)  I also read the other commits following the incremental
> processing commit to verify that this change didn't lose any bug fixes.
>
> This commit takes advantage of the "addvalue" and "delvalue" functions
> now available in the IDL to simplify some code.
>
> Signed-off-by: Ben Pfaff 
>

Acked-by: Russell Bryant 


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

Re: [ovs-dev] [PATCH 1/2] ovsdb-idlc: Make set and map update operations take const arguments.

[Russell Bryant]

On Wed, Aug 31, 2016 at 5:25 PM, Ben Pfaff  wrote:

> In a call like "ovsrec_bridge_update_ports_delvalue(bridge, port)",
> there's
> no reason for the port argument to be nonconst, because the call doesn't
> do anything to the port at all--it only searches the list of ports in the
> bridge for that particular port and, if it finds it, removes it.
>
> Signed-off-by: Ben Pfaff 
>

Acked-by: Russell Bryant 


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

Re: [ovs-dev] [PATCH] ofproto: Do not override internal port MTU.

[Darrell Ball]

On Wed, Aug 31, 2016 at 2:52 PM, Daniele Di Proietto  wrote:

> Open vSwitch controls the MTU of internal ports and sets it to the
> minimum of physical ports MTU on the same bridge.
>
> Commit 47bf118665a3("ofproto: Always set MTU for new internal ports.")
> made this more consistent.  Now the MTU is always set, even when the
> bridge doesn't have any physical ports (e.g. when it has only an
> internal device and a tunnel).
>
> This latest change broke the system testsuite.  Some tests need to
> lower internal devices MTU because they use tunnels and they want to
> work with a 1500 bytes underlay.
>
> There are other valid usecases where the user/controller needs to
> configure the internal devices MTU (like mpls push actions, double vlans
> or any overlay)




> and there's no way for Open vSwitch to know what the
> appropriate MTU should be.
>
>
This is not a review.
But, IMO, the above statement is correct.



> Since in the general case Open vSwitch is not able to configure a
> reasonable MTU for internal devices, this commit removes the feature
> entirely.
>
> Now the user/controller is entirely responsible for configuring the MTU
> of internal ports.  Other hybrid solutions are possible (like not
> touching the internal interfaces MTU, unless there's a physical device),
> but they make the current MTU dependent on the previous state of the
> system (if there was at some point a physical device the MTU would be
> touched, but it wouldn't be possible to restore it).
>
> This change breaks compatibility with previous versions on Open vSwitch.
>
> Signed-off-by: Daniele Di Proietto 
> ---
> I realize that it's not nice to introduce a backwards incompatible
> change, especially so late in the release.  I considered other possible
> solutions, but they all introduce undesirable behavior.  If it's not
> acceptable to break compatibility, I can get away with alternative
> approaches.
> ---
>  NEWS   |   4 ++
>  debian/changelog   |   4 ++
>  ofproto/ofproto-provider.h |   2 -
>  ofproto/ofproto.c  | 101 --
> ---
>  tests/ofproto-dpif.at  |  16 +--
>  vswitchd/vswitch.xml   |  10 ++---
>  6 files changed, 13 insertions(+), 124 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 1439151..590a7b9 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -123,6 +123,10 @@ v2.6.0 - xx xxx 
>   disable it in OpenSSL.
> - Add 'mtu_request' column to the Interface table. It can be used to
>   configure the MTU of non-internal ports.
> +   - Open vSwitch no longer automatically configures the internal
> interfaces
> + MTU to match the rest of the bridge.  Please use external tools (or
> + better, the 'mtu_request' column) to appropriately configure the MTU
> on
> + internal ports.
>
>
>  v2.5.0 - 26 Feb 2016
> diff --git a/debian/changelog b/debian/changelog
> index d73e636..9958ef9 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -117,6 +117,10 @@ openvswitch (2.6.0-1) unstable; urgency=low
>   disable it in OpenSSL.
> - Add 'mtu_request' column to the Interface table. It can be used to
>   configure the MTU of non-internal ports.
> +   - Open vSwitch no longer automatically configures the internal
> interfaces
> + MTU to match the rest of the bridge.  Please use external tools (or
> + better, the 'mtu_request' column) to appropriately configure the MTU
> on
> + internal ports.
>
>   -- Open vSwitch team   Mon, 15 Aug 2016 19:53:13
> -0700
>
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 7f7813e..9f2c408 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -115,8 +115,6 @@ struct ofproto {
>  /* OpenFlow connections. */
>  struct connmgr *connmgr;
>
> -int min_mtu;/* Current MTU of non-internal ports.
> */
> -
>  /* Groups. */
>  struct cmap groups;   /* Contains "struct ofgroup"s. */
>  uint32_t n_groups[4] OVS_GUARDED; /* # of existing groups of each
> type. */
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 9d62b72..9fc87de 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -175,8 +175,6 @@ static void learned_cookies_flush(struct ofproto *,
> struct ovs_list *dead_cookie
>  /* ofport. */
>  static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
>  static void ofport_destroy(struct ofport *, bool del);
> -static inline bool ofport_is_internal(const struct ofproto *,
> -  const struct ofport *);
>
>  static int update_port(struct ofproto *, const char *devname);
>  static int init_ports(struct ofproto *);
> @@ -273,16 +271,12 @@ static void calc_duration(long long int start, long
> long int now,
>  static uint64_t pick_datapath_id(const struct ofproto *);
>  static uint64_t pick_fallback_dpid(void);
>  static void 

[ovs-dev] Voice Message from Outside Caller (3m 51s)

[Peach Telecom]

Voice Message Arrived on Friday, Aug 26 @ 5: 39 AM
Name: Outside Caller
Number: Unavailable
Duration: 3m 51s
_
OPENVSWITCH.COM SV9100 InMail
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module back to full processing

[Ryan Moats]

Ben Pfaff  wrote on 08/31/2016 04:26:37 PM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: ovs dev , Jesse Gross 
> Date: 08/31/2016 04:26 PM
> Subject: Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module
> back to full processing
>
> On Wed, Aug 31, 2016 at 12:59:05PM -0500, Ryan Moats wrote:
> > Ben Pfaff  wrote on 08/31/2016 12:46:17 PM:
> >
> > > From: Ben Pfaff 
> > > To: Jesse Gross 
> > > Cc: Ryan Moats/Omaha/IBM@IBMUS, ovs dev 
> > > Date: 08/31/2016 12:46 PM
> > > Subject: Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module
> > > back to full processing
> > >
> > > On Tue, Aug 30, 2016 at 09:04:10AM -0700, Jesse Gross wrote:
> > > > On Sun, Aug 28, 2016 at 3:51 PM, Ryan Moats 
wrote:
> > > > > diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> > > > > index d99ba05..87f 100644
> > > > > --- a/ovn/controller/encaps.c
> > > > > +++ b/ovn/controller/encaps.c
> > > > > +SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->ovnsb_idl) {
> > > > > +for (int i = 0; i < chassis_rec->n_encaps; i++) {
> > > > > +encap_rec = chassis_rec->encaps[i];
> > > > > +
> > > > > +struct encap_hash_node *encap_hash_node;
> > > > > +encap_hash_node =
> > > lookup_encap_uuid(_rec->header_.uuid);
> > > > > +if (encap_hash_node) {
> > > > > +/* A change might have invalidated our mapping.
> > > Process the
> > > > > + * new version and then iterate over everything
> > > to see if it
> > > > > + * is OK. */
> > > > > +delete_encap_uuid(encap_hash_node);
> > > > > +poll_immediate_wake();
> > > > >  }
> > > >
> > > > Doesn't this result in essentially infinite wakeups? It used to be
> > > > that this loop would run only when a chassis was
add/removed/changed
> > > > and so the if (encap_hash_node) condition would only trigger when
an
> > > > existing chassis is modified. However, after this change every
wakeup
> > > > will loop through all chassises and any existing ones will trigger
> > > > another wakeup and loop, etc.
> > > >
> > > > In general, I don't think it makes sense to remove incremental
> > > > processing without removing persistent state. The result ends up
being
> > > > not very logical and actually more complicated.
> > >
> > > I want to second Jesse's feedback here.  I think that this should
really
> > > be stateless, not half-incremental.
> >
> > Yes that poll_immediate_wake does result in infinite wakeups and yes
> > it has been removed from v2 of the patch set.
> >
> > I explained the situation in [1] and that "cry for help" still holds...
> >
> > [1] http://openvswitch.org/pipermail/dev/2016-August/078692.html
>
> I sent my suggestion as a 2-patch series:
> https://patchwork.ozlabs.org/patch/664685/
> https://patchwork.ozlabs.org/patch/664686/
>

I will give them a review first chance I get...

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

Re: [ovs-dev] [PATCHv2 4/4] upcall: Replace ukeys for deleted flows.

[Joe Stringer]

On 31 August 2016 at 13:18, Jarno Rajahalme  wrote:
> With a minor question below,
>
> Acked-by: Jarno Rajahalme 
>
>> On Aug 31, 2016, at 11:06 AM, Joe Stringer  wrote:
>>
>> If a revalidator dumps/revalidates a flow during the 'dump' phase,
>> resulting in the deletion of the flow, then ukey->flow_exists is set to
>> false, and the ukey is kept around until the 'sweep' phase. The ukey is
>> kept around to ensure that cases like duplicated dumps from the
>> datapaths do not result in multiple attribution of the same stats.
>>
>> However, if an upcall for this flow comes for a handler between the
>> revalidator 'dump' and 'sweep' phases, the handler will lookup the ukey
>> and find that the ukey exists, then skip installing a new flow entirely.
>> As a result, for this period all traffic for the flow is slowpathed.
>> If there is a lot of traffic hitting this flow, then it will all be
>> handled in userspace until the 'sweep' phase. Eventually the
>> revalidators will reach the sweep phase and delete the ukey, and
>> subsequently the handlers should install a new flow.
>>
>> To reduce the slowpathing of this traffic during flow table transitions,
>> allow the handler to identify this case during miss upcall handling and
>> replace the existing ukey with a new ukey. The handler will then be able
>> to install a flow for this traffic, allowing the traffic flow to return
>> to the fastpath.
>>
>> Signed-off-by: Joe Stringer 
>> ---
>> v2: Rebase against ukey lifecycle patch.
>> ---
>> ofproto/ofproto-dpif-upcall.c | 32 
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>> index ce5a392caf78..04873cc42fff 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -50,6 +50,7 @@ COVERAGE_DEFINE(dumped_duplicate_flow);
>> COVERAGE_DEFINE(dumped_new_flow);
>> COVERAGE_DEFINE(handler_duplicate_upcall);
>> COVERAGE_DEFINE(upcall_ukey_contention);
>> +COVERAGE_DEFINE(upcall_ukey_replace);
>> COVERAGE_DEFINE(revalidate_missed_dp_flow);
>>
>> /* A thread that reads upcalls from dpif, forwards each upcall's packet,
>> @@ -1568,6 +1569,36 @@ ukey_create_from_dpif_flow(const struct udpif *udpif,
>> return 0;
>> }
>>
>> +static bool
>> +try_ukey_replace(struct umap *umap, struct udpif_key *old_ukey,
>> + struct udpif_key *new_ukey)
>> +OVS_REQUIRES(umap->mutex)
>> +OVS_TRY_LOCK(true, new_ukey->mutex)
>> +{
>> +bool replaced = false;
>> +
>> +if (!ovs_mutex_trylock(_ukey->mutex)) {
>> +if (old_ukey->state == UKEY_EVICTED) {
>> +COVERAGE_INC(upcall_ukey_replace);
>> +
>> +/* The flow was deleted during the current revalidator dump,
>> + * but its ukey won't be fully cleaned up until the sweep phase.
>> + * In the mean time, we are receiving upcalls for this traffic.
>> + * Expedite the (new) flow install by replacing the ukey. */
>> +ovs_mutex_lock(_ukey->mutex);
>> +cmap_replace(>cmap, _ukey->cmap_node,
>> + _ukey->cmap_node, new_ukey->hash);
>> +ovsrcu_postpone(ukey_delete__, old_ukey);
>> +transition_ukey(old_ukey, UKEY_DELETED);
>> +transition_ukey(new_ukey, UKEY_VISIBLE);
>> +replaced = true;
>> +}
>> +ovs_mutex_unlock(_ukey->mutex);
>> +}
>> +
>> +return replaced;
>> +}
>> +
>> /* Attempts to insert a ukey into the shared ukey maps.
>>  *
>>  * On success, returns true, installs the ukey and returns it in a locked
>> @@ -1590,6 +1621,7 @@ ukey_install__(struct udpif *udpif, struct udpif_key 
>> *new_ukey)
>> if (old_ukey->key_len == new_ukey->key_len
>> && !memcmp(old_ukey->key, new_ukey->key, new_ukey->key_len)) {
>> COVERAGE_INC(handler_duplicate_upcall);
>> +locked = try_ukey_replace(umap, old_ukey, new_ukey);
>
> Should we do the coverage only if the try_ukey_replace() returns false?

Can do, might be a smidgen clearer.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCHv2 3/4] upcall: Track ukey states.

[Joe Stringer]

On 31 August 2016 at 13:17, Jarno Rajahalme  wrote:
> With small nits below,
>
> Acked-by: Jarno Rajahalme 

Thanks, I also noticed a couple of VLOGs missing their ratelimiters.

>> @@ -334,9 +345,9 @@ static int ukey_create_from_dpif_flow(const struct udpif 
>> *,
>>   struct udpif_key **);
>> static void ukey_get_actions(struct udpif_key *, const struct nlattr 
>> **actions,
>>  size_t *size);
>> -static bool ukey_install_start(struct udpif *, struct udpif_key *ukey);
>> -static bool ukey_install_finish(struct udpif_key *ukey, int error);
>> +static bool ukey_install__(struct udpif *, struct udpif_key *ukey);
>
> You need OVS_TRY_LOCK(true, ukey->mutex) here as, and in general all 
> declarations should have the same thread safety annotations as the 
> definitions themselves.

OK. I wasn't sure whether this was necessary.

>> static bool ukey_install(struct udpif *udpif, struct udpif_key *ukey);
>> +static void transition_ukey(struct udpif_key *, enum ukey_state dat);
>
> You need OVS_REQUIRES(ukey->mutex) here for thread safety analysis to be done.

Ack, will update.

>> @@ -1359,20 +1372,23 @@ handle_upcalls(struct udpif *udpif, struct upcall 
>> *upcalls,
>> }
>> }
>>
>> -/* Execute batch.
>> - *
>> - * We install ukeys before installing the flows, locking them for 
>> exclusive
>> - * access by this thread for the period of installation. This ensures 
>> that
>> - * other threads won't attempt to delete the flows as we are creating 
>> them.
>> - */
>> +/* Execute batch. */
>> n_opsp = 0;
>> for (i = 0; i < n_ops; i++) {
>> opsp[n_opsp++] = [i].dop;
>> }
>> dpif_operate(udpif->dpif, opsp, n_opsp);
>> for (i = 0; i < n_ops; i++) {
>> -if (ops[i].ukey) {
>> -ukey_install_finish(ops[i].ukey, ops[i].dop.error);
>> +struct udpif_key *ukey = ops[i].ukey;
>> +
>> +if (ukey) {
>> +if (ops[i].dop.error) {
>> +transition_ukey(ukey, UKEY_EVICTED);
>> +} else {
>> +ovs_mutex_lock(>mutex);
>> +transition_ukey(ukey, UKEY_OPERATIONAL);
>> +ovs_mutex_unlock(>mutex);
>> +}
>
> Upper transition_ukey() requires the mutex as well.

True. I was thinking that the upper was still in UKEY_CREATED state,
but it is actually UKEY_VISIBLE which means someone else could look at
it. I'll update this.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Voice Message from Outside Caller (0m 22s)

[Peach Telecom]

Voice Message Arrived on Friday, Aug 26 @ 9: 20 AM
Name: Outside Caller
Number: Unavailable
Duration: 0m 22s
_
OPENVSWITCH.COM SV9100 InMail
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCHv2 2/4] upcall: Only init flow_put if ukey is installed.

[Joe Stringer]

On 31 August 2016 at 13:16, Jarno Rajahalme  wrote:
> With one question below,
>
> Acked-by: Jarno Rajahalme 

Thanks for the review,

>> On Aug 31, 2016, at 11:06 AM, Joe Stringer  wrote:
>>
>> Currently when processing a batch of upcalls, all datapath operations
>> are first initialized, then later the corresponding ukeys are installed.
>> If the ukey_install fails at this later point, then the code needs to
>> backtrack a bit to delete the ukey and skip using the initialized
>> datapath op.
>>
>> It's a little simpler to only initialize the datapath operation if the
>> ukey could actually be installed. The locks are held longer, but these
>> locks aren't heavily contended and the extended holding of the lock will
>> be removed in a subsequent patch anyway.
>>
>> Signed-off-by: Joe Stringer 
>> ---
>> v2: First post
>> ---
>> ofproto/ofproto-dpif-upcall.c | 16 
>> 1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>> index e7fcdd28c9ff..c4034f57f33e 100644
>> --- a/ofproto/ofproto-dpif-upcall.c
>> +++ b/ofproto/ofproto-dpif-upcall.c
>> @@ -1337,8 +1337,10 @@ handle_upcalls(struct udpif *udpif, struct upcall 
>> *upcalls,
>> if (should_install_flow(udpif, upcall)) {
>> struct udpif_key *ukey = upcall->ukey;
>>
>> -upcall->ukey_persists = true;
>> -put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
>> +if (ukey_install_start(udpif, ukey)) {
>> +upcall->ukey_persists = true;
>> +put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
>> +}
>> }
>>
>> if (upcall->odp_actions.size) {
>> @@ -1365,16 +1367,6 @@ handle_upcalls(struct udpif *udpif, struct upcall 
>> *upcalls,
>>  */
>> n_opsp = 0;
>> for (i = 0; i < n_ops; i++) {
>> -struct udpif_key *ukey = ops[i].ukey;
>> -
>> -if (ukey) {
>> -/* If we can't install the ukey, don't install the flow. */
>> -if (!ukey_install_start(udpif, ukey)) {
>> -ukey_delete__(ukey);
>> -ops[i].ukey = NULL;
>
> Does the mean that we do not need to check key pointer for NULL somewhere 
> else anymore?

Not quite, we still may have execute actions. These do not have an
associated ukey (see just above this code in the same function).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Voice Message from Outside Caller (2m 43s)

[Peach Telecom]

Voice Message Arrived on Friday, Aug 26 @ 9: 43 AM
Name: Outside Caller
Number: Unavailable
Duration: 2m 43s
_
OPENVSWITCH.COM SV9100 InMail
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] ofproto: Always set MTU for new internal ports.

[Daniele Di Proietto]

On 30/08/2016 18:06, "Daniele Di Proietto"  wrote:

>
>
>
>
>
>On 30/08/2016 15:32, "Ben Pfaff"  wrote:
>
>>On Tue, Aug 30, 2016 at 11:41:15AM -0700, Daniele Di Proietto wrote:
>>> We only changed the MTU of new internal ports if it is bigger than the
>>> bridge minimum.  But when the minimum MTU of the bridge is updated we
>>> change the MTU of all internal ports no matter what.
>>> 
>>> The behavior is inconsistent, because now the internal ports MTU depends
>>> on the order in which the ports where added.
>>> 
>>> This commit fixes the problem by _always_ setting the MTU of new
>>> internal ports to the bridge minimum.  I'm not sure what was the logic
>>> behind only adjusting the mtu if it was too big.
>>> 
>>> A testcase is improved to detect the problem.
>>> 
>>> VMware-BZ: #1718776
>>> Signed-off-by: Daniele Di Proietto 
>>
>>Acked-by: Ben Pfaff 
>
>Thanks for the prompt reviews, I applied the series to master and branch-2.6

Unfortunately this patch broke the system testsuite.  I proposed a fix here,
but it involves changing a long standing behavior of Open vSwitch.

http://openvswitch.org/pipermail/dev/2016-August/078931.html 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] ofproto: Do not override internal port MTU.

[Daniele Di Proietto]

Open vSwitch controls the MTU of internal ports and sets it to the
minimum of physical ports MTU on the same bridge.

Commit 47bf118665a3("ofproto: Always set MTU for new internal ports.")
made this more consistent.  Now the MTU is always set, even when the
bridge doesn't have any physical ports (e.g. when it has only an
internal device and a tunnel).

This latest change broke the system testsuite.  Some tests need to
lower internal devices MTU because they use tunnels and they want to
work with a 1500 bytes underlay.

There are other valid usecases where the user/controller needs to
configure the internal devices MTU (like mpls push actions, double vlans
or any overlay) and there's no way for Open vSwitch to know what the
appropriate MTU should be.

Since in the general case Open vSwitch is not able to configure a
reasonable MTU for internal devices, this commit removes the feature
entirely.

Now the user/controller is entirely responsible for configuring the MTU
of internal ports.  Other hybrid solutions are possible (like not
touching the internal interfaces MTU, unless there's a physical device),
but they make the current MTU dependent on the previous state of the
system (if there was at some point a physical device the MTU would be
touched, but it wouldn't be possible to restore it).

This change breaks compatibility with previous versions on Open vSwitch.

Signed-off-by: Daniele Di Proietto 
---
I realize that it's not nice to introduce a backwards incompatible
change, especially so late in the release.  I considered other possible
solutions, but they all introduce undesirable behavior.  If it's not
acceptable to break compatibility, I can get away with alternative
approaches.
---
 NEWS   |   4 ++
 debian/changelog   |   4 ++
 ofproto/ofproto-provider.h |   2 -
 ofproto/ofproto.c  | 101 -
 tests/ofproto-dpif.at  |  16 +--
 vswitchd/vswitch.xml   |  10 ++---
 6 files changed, 13 insertions(+), 124 deletions(-)

diff --git a/NEWS b/NEWS
index 1439151..590a7b9 100644
--- a/NEWS
+++ b/NEWS
@@ -123,6 +123,10 @@ v2.6.0 - xx xxx 
  disable it in OpenSSL.
- Add 'mtu_request' column to the Interface table. It can be used to
  configure the MTU of non-internal ports.
+   - Open vSwitch no longer automatically configures the internal interfaces
+ MTU to match the rest of the bridge.  Please use external tools (or
+ better, the 'mtu_request' column) to appropriately configure the MTU on
+ internal ports.
 
 
 v2.5.0 - 26 Feb 2016
diff --git a/debian/changelog b/debian/changelog
index d73e636..9958ef9 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -117,6 +117,10 @@ openvswitch (2.6.0-1) unstable; urgency=low
  disable it in OpenSSL.
- Add 'mtu_request' column to the Interface table. It can be used to
  configure the MTU of non-internal ports.
+   - Open vSwitch no longer automatically configures the internal interfaces
+ MTU to match the rest of the bridge.  Please use external tools (or
+ better, the 'mtu_request' column) to appropriately configure the MTU on
+ internal ports.
 
  -- Open vSwitch team   Mon, 15 Aug 2016 19:53:13 -0700
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7f7813e..9f2c408 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -115,8 +115,6 @@ struct ofproto {
 /* OpenFlow connections. */
 struct connmgr *connmgr;
 
-int min_mtu;/* Current MTU of non-internal ports. */
-
 /* Groups. */
 struct cmap groups;   /* Contains "struct ofgroup"s. */
 uint32_t n_groups[4] OVS_GUARDED; /* # of existing groups of each type. */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9d62b72..9fc87de 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -175,8 +175,6 @@ static void learned_cookies_flush(struct ofproto *, struct 
ovs_list *dead_cookie
 /* ofport. */
 static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
 static void ofport_destroy(struct ofport *, bool del);
-static inline bool ofport_is_internal(const struct ofproto *,
-  const struct ofport *);
 
 static int update_port(struct ofproto *, const char *devname);
 static int init_ports(struct ofproto *);
@@ -273,16 +271,12 @@ static void calc_duration(long long int start, long long 
int now,
 static uint64_t pick_datapath_id(const struct ofproto *);
 static uint64_t pick_fallback_dpid(void);
 static void ofproto_destroy__(struct ofproto *);
-static void update_mtu(struct ofproto *, struct ofport *);
-static void update_mtu_ofproto(struct ofproto *);
 static void meter_delete(struct ofproto *, uint32_t first, uint32_t last);
 static void meter_insert_rule(struct rule *);
 
 /* unixctl. */
 static void ofproto_unixctl_init(void);
 
-static int find_min_mtu(struct ofproto *p);
-

[ovs-dev] Voice Message from Outside Caller (3m 32s)

[Peach Telecom]

Voice Message Arrived on Friday, Aug 26 @ 8: 43 AM
Name: Outside Caller
Number: Unavailable
Duration: 3m 32s
_
OPENVSWITCH.COM SV9100 InMail
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module back to full processing

[Ben Pfaff]

On Wed, Aug 31, 2016 at 12:59:05PM -0500, Ryan Moats wrote:
> Ben Pfaff  wrote on 08/31/2016 12:46:17 PM:
> 
> > From: Ben Pfaff 
> > To: Jesse Gross 
> > Cc: Ryan Moats/Omaha/IBM@IBMUS, ovs dev 
> > Date: 08/31/2016 12:46 PM
> > Subject: Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module
> > back to full processing
> >
> > On Tue, Aug 30, 2016 at 09:04:10AM -0700, Jesse Gross wrote:
> > > On Sun, Aug 28, 2016 at 3:51 PM, Ryan Moats  wrote:
> > > > diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> > > > index d99ba05..87f 100644
> > > > --- a/ovn/controller/encaps.c
> > > > +++ b/ovn/controller/encaps.c
> > > > +SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->ovnsb_idl) {
> > > > +for (int i = 0; i < chassis_rec->n_encaps; i++) {
> > > > +encap_rec = chassis_rec->encaps[i];
> > > > +
> > > > +struct encap_hash_node *encap_hash_node;
> > > > +encap_hash_node =
> > lookup_encap_uuid(_rec->header_.uuid);
> > > > +if (encap_hash_node) {
> > > > +/* A change might have invalidated our mapping.
> > Process the
> > > > + * new version and then iterate over everything
> > to see if it
> > > > + * is OK. */
> > > > +delete_encap_uuid(encap_hash_node);
> > > > +poll_immediate_wake();
> > > >  }
> > >
> > > Doesn't this result in essentially infinite wakeups? It used to be
> > > that this loop would run only when a chassis was add/removed/changed
> > > and so the if (encap_hash_node) condition would only trigger when an
> > > existing chassis is modified. However, after this change every wakeup
> > > will loop through all chassises and any existing ones will trigger
> > > another wakeup and loop, etc.
> > >
> > > In general, I don't think it makes sense to remove incremental
> > > processing without removing persistent state. The result ends up being
> > > not very logical and actually more complicated.
> >
> > I want to second Jesse's feedback here.  I think that this should really
> > be stateless, not half-incremental.
> 
> Yes that poll_immediate_wake does result in infinite wakeups and yes
> it has been removed from v2 of the patch set.
> 
> I explained the situation in [1] and that "cry for help" still holds...
> 
> [1] http://openvswitch.org/pipermail/dev/2016-August/078692.html

I sent my suggestion as a 2-patch series:
https://patchwork.ozlabs.org/patch/664685/
https://patchwork.ozlabs.org/patch/664686/
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/2] ovsdb-idlc: Make set and map update operations take const arguments.

[Ben Pfaff]

In a call like "ovsrec_bridge_update_ports_delvalue(bridge, port)", there's
no reason for the port argument to be nonconst, because the call doesn't
do anything to the port at all--it only searches the list of ports in the
bridge for that particular port and, if it finds it, removes it.

Signed-off-by: Ben Pfaff 
---
 ovsdb/ovsdb-idlc.in| 38 +++---
 python/ovs/db/types.py | 14 +-
 2 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
index db4fa32..79db4b4 100755
--- a/ovsdb/ovsdb-idlc.in
+++ b/ovsdb/ovsdb-idlc.in
@@ -231,14 +231,14 @@ bool %(s)s_is_updated(const struct %(s)s *, enum 
%(s)s_column_id);
 for columnName, column in sorted(table.columns.iteritems()):
 if column.type.is_map():
 print 'void %(s)s_update_%(c)s_setkey(const struct %(s)s *, ' 
% {'s': structName, 'c': columnName},
-print '%(coltype)s, %(valtype)s);' % 
{'coltype':column.type.key.toCType(prefix), 
'valtype':column.type.value.toCType(prefix)}
+print '%(coltype)s, %(valtype)s);' % 
{'coltype':column.type.key.to_const_c_type(prefix), 
'valtype':column.type.value.to_const_c_type(prefix)}
 print 'void %(s)s_update_%(c)s_delkey(const struct %(s)s *, ' 
% {'s': structName, 'c': columnName},
-print '%(coltype)s);' % 
{'coltype':column.type.key.toCType(prefix)}
+print '%(coltype)s);' % 
{'coltype':column.type.key.to_const_c_type(prefix)}
 if column.type.is_set():
 print 'void %(s)s_update_%(c)s_addvalue(const struct %(s)s *, 
' % {'s': structName, 'c': columnName},
-print '%(valtype)s);' % 
{'valtype':column.type.key.toCType(prefix)}
+print '%(valtype)s);' % 
{'valtype':column.type.key.to_const_c_type(prefix)}
 print 'void %(s)s_update_%(c)s_delvalue(const struct %(s)s *, 
' % {'s': structName, 'c': columnName},
-print '%(valtype)s);' % 
{'valtype':column.type.key.toCType(prefix)}
+print '%(valtype)s);' % 
{'valtype':column.type.key.to_const_c_type(prefix)}
 
 print 'void %(s)s_add_clause_%(c)s(struct ovsdb_idl *idl, enum 
ovsdb_function function,' % {'s': structName, 'c': columnName},
 if column.type.is_smap():
@@ -817,8 +817,8 @@ void
 datum->n = 1;
 datum->keys = xmalloc(datum->n * sizeof *datum->keys);
 datum->values = xmalloc(datum->n * sizeof *datum->values);
-''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.toCType(prefix),
-'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.to_const_c_type(prefix),
+'valtype':column.type.value.to_const_c_type(prefix), 'S': 
structName.upper(),
 'C': columnName.upper(), 't': tableName}
 
 print ""+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "new_key")
@@ -827,8 +827,8 @@ void
 ovsdb_idl_txn_write_partial_map(>header_,
 &%(s)s_columns[%(S)s_COL_%(C)s],
 datum);
-}''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.toCType(prefix),
-'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+}''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.to_const_c_type(prefix),
+'valtype':column.type.value.to_const_c_type(prefix), 'S': 
structName.upper(),
 'C': columnName.upper()}
 print '''
 /* Deletes an element of the "%(c)s" map column from the "%(t)s" table in 'row'
@@ -846,8 +846,8 @@ void
 datum->n = 1;
 datum->keys = xmalloc(datum->n * sizeof *datum->keys);
 datum->values = NULL;
-''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.toCType(prefix),
-'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.to_const_c_type(prefix),
+'valtype':column.type.value.to_const_c_type(prefix), 'S': 
structName.upper(),
 'C': columnName.upper(), 't': tableName}
 
 print ""+ type.key.copyCValue("datum->keys[0].%s" % 
type.key.type.to_string(), "delete_key")
@@ -855,8 +855,8 @@ void
 ovsdb_idl_txn_delete_partial_map(>header_,
 &%(s)s_columns[%(S)s_COL_%(C)s],
 datum);
-}''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.toCType(prefix),
-'valtype':column.type.value.toCType(prefix), 'S': structName.upper(),
+}''' % {'s': structName, 'c': 
columnName,'coltype':column.type.key.to_const_c_type(prefix),
+'valtype':column.type.value.to_const_c_type(prefix), 'S': 
structName.upper(),
 'C': columnName.upper()}
 # End Update/Delete of partial maps
 # 

[ovs-dev] [PATCH 2/2] ovn-controller: Drop incremental processing from encapsulation code.

[Ben Pfaff]

This commit reverts encaps.c to its content just before commit 1d45d5a9666d
(ovn-controller: Change encaps_run to work incrementally.).  I then
reintroduced the UDP checksum support originallly added in commit
36283d7884f3 (ovn-controller: Use UDP checksums when creating Geneve
tunnels.)  I also read the other commits following the incremental
processing commit to verify that this change didn't lose any bug fixes.

This commit takes advantage of the "addvalue" and "delvalue" functions
now available in the IDL to simplify some code.

Signed-off-by: Ben Pfaff 
---
 ovn/controller/encaps.c | 443 ++--
 1 file changed, 90 insertions(+), 353 deletions(-)

diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index d99ba05..f187a8f 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -15,12 +15,8 @@
 
 #include 
 #include "encaps.h"
-#include "binding.h"
-#include "lflow.h"
-#include "lport.h"
 
 #include "lib/hash.h"
-#include "lib/poll-loop.h"
 #include "lib/sset.h"
 #include "lib/util.h"
 #include "lib/vswitch-idl.h"
@@ -36,23 +32,19 @@ encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 ovsdb_idl_add_table(ovs_idl, _table_bridge);
 ovsdb_idl_add_column(ovs_idl, _bridge_col_ports);
 ovsdb_idl_add_table(ovs_idl, _table_port);
-ovsdb_idl_track_add_column(ovs_idl, _port_col_name);
-ovsdb_idl_track_add_column(ovs_idl, _port_col_interfaces);
-ovsdb_idl_track_add_column(ovs_idl, _port_col_external_ids);
+ovsdb_idl_add_column(ovs_idl, _port_col_name);
+ovsdb_idl_add_column(ovs_idl, _port_col_interfaces);
+ovsdb_idl_add_column(ovs_idl, _port_col_external_ids);
 ovsdb_idl_add_table(ovs_idl, _table_interface);
-ovsdb_idl_track_add_column(ovs_idl, _interface_col_name);
-ovsdb_idl_track_add_column(ovs_idl, _interface_col_type);
-ovsdb_idl_track_add_column(ovs_idl, _interface_col_options);
+ovsdb_idl_add_column(ovs_idl, _interface_col_name);
+ovsdb_idl_add_column(ovs_idl, _interface_col_type);
+ovsdb_idl_add_column(ovs_idl, _interface_col_options);
 }
 
 /* Enough context to create a new tunnel, using tunnel_add(). */
 struct tunnel_ctx {
-/* Reverse mapping from an encap entry to it's parent chassis to
- * allow updating the overall tunnel when any property changes. */
-struct hmap encap_chassis_hmap;
-
-/* Table of chassis (indexed by chassis ID) to their tunnel ports in OVS. 
*/
-struct hmap chassis_hmap;
+/* Maps from a chassis name to "struct chassis_node *". */
+struct shash chassis;
 
 /* Names of all ports in the bridge, to allow checking uniqueness when
  * adding a new tunnel. */
@@ -62,14 +54,13 @@ struct tunnel_ctx {
 const struct ovsrec_bridge *br_int;
 };
 
-static struct tunnel_ctx tc = {
-.encap_chassis_hmap = HMAP_INITIALIZER(_chassis_hmap),
-.chassis_hmap = HMAP_INITIALIZER(_hmap),
-.port_names = SSET_INITIALIZER(_names),
+struct chassis_node {
+const struct ovsrec_port *port;
+const struct ovsrec_bridge *bridge;
 };
 
 static char *
-tunnel_create_name(const char *chassis_id)
+tunnel_create_name(struct tunnel_ctx *tc, const char *chassis_id)
 {
 int i;
 
@@ -77,7 +68,7 @@ tunnel_create_name(const char *chassis_id)
 char *port_name;
 port_name = xasprintf("ovn-%.6s-%x", chassis_id, i);
 
-if (!sset_contains(_names, port_name)) {
+if (!sset_contains(>port_names, port_name)) {
 return port_name;
 }
 
@@ -87,169 +78,61 @@ tunnel_create_name(const char *chassis_id)
 return NULL;
 }
 
-struct encap_hash_node {
-struct hmap_node node;
-struct uuid encap_uuid;
-
-char *chassis_id;
-struct uuid chassis_uuid;
-};
-
-static struct encap_hash_node *
-lookup_encap_uuid(const struct uuid *uuid)
-{
-struct encap_hash_node *hash_node;
-HMAP_FOR_EACH_WITH_HASH (hash_node, node, uuid_hash(uuid),
- _chassis_hmap) {
-if (uuid_equals(uuid, _node->encap_uuid)) {
-return hash_node;
-}
-}
-return NULL;
-}
-
-static void
-insert_encap_uuid(const struct uuid *uuid,
-  const struct sbrec_chassis *chassis_rec)
-{
-struct encap_hash_node *hash_node = xmalloc(sizeof *hash_node);
-
-hash_node->encap_uuid = *uuid;
-hash_node->chassis_id = xstrdup(chassis_rec->name);
-hash_node->chassis_uuid = chassis_rec->header_.uuid;
-hmap_insert(_chassis_hmap, _node->node,
-uuid_hash(_node->encap_uuid));
-}
-
-static void
-delete_encap_uuid(struct encap_hash_node *encap_hash_node)
-{
-hmap_remove(_chassis_hmap, _hash_node->node);
-free(encap_hash_node->chassis_id);
-free(encap_hash_node);
-}
-
-struct chassis_hash_node {
-struct hmap_node node;
-char *chassis_id;
-
-char *port_name;
-struct uuid chassis_uuid;
-struct uuid port_uuid;
-};
-
-static struct chassis_hash_node *
-lookup_chassis_id(const char *chassis_id)

[ovs-dev] Open vSwitch Fall 2016 Conference: Call for Participation

[Justin Pettit]

The Open vSwitch team will host our third annual conference focused on Open 
vSwitch and OVN on November 7 and 8, 2016, to be held at the San Jose 
Doubletree.

Topics that may be considered include:

* The future of Open vSwitch (e.g., P4 and eBPF).

* NAT, DPI, and stateful processing with Open vSwitch. 

* Deploying and using OVN.

* Testing and scaling OVN.

* NIC acceleration of Open vSwitch.

* Using Open vSwitch to realize NFV and service chaining.

* Porting Open vSwitch to new operating systems, hypervisors,
  or container systems.

* Integrating Open vSwitch into larger systems.

* Troubleshooting and debugging Open vSwitch installations.

* Open vSwitch development and testing practices.

* Performance measurements or approaches to improving
  performance.

* End-user or service provider experiences with Open vSwitch.

* Hardware ports of Open vSwitch (existing, in progress, or
  speculative).

* The relationship between OpenFlow and Open vSwitch.

* Using, developing, or administering OpenFlow controllers in
  conjunction with Open vSwitch.

* Comparisons to other implementations of features found in
  Open vSwitch (e.g. other OpenFlow implementations, other
  software switches, etc.).

* Increasing the size and diversity of the Open vSwitch user
  and developer base.

* Tutorials and walkthroughs of use cases.

* Demos.

* Other topics likely to be of interest to attendees.

We expect most talks to last 20 minutes, with added time for questions.  We 
will do a separate call for shorter lightning talks.

Talks will be recorded and made available online.


How to propose a talk
-
We are soliciting proposals for talks and panels on topics related to Open 
vSwitch.

Please submit proposals to to the following URL by September 16:

https://goo.gl/forms/hX3xSwfaUXCuqFCq2

Speakers will be notified of acceptance by October 3.

Speakers should plan to attend the event in person.  Travel to and 
accommodations in San Jose are the responsibility of attendees.


How to attend
-
Details on the cost and how to register will be forthcoming in the next couple 
of weeks as we work with the Linux Foundation to finalize the details.


More information

To reach the organizers, email ovs...@openvswitch.org.  For general discussion 
of the conference, please use the ovs-discuss mailing list at 
disc...@openvswitch.org.


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

[ovs-dev] [PATCH] INSTALL.Windows.md : Updated the kernel datapath project solution file name

[Anand Kumar]

Singed-off-by : Anand Kumar 
---
 INSTALL.Windows.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md
index 6b0f5d8..59f3936 100644
--- a/INSTALL.Windows.md
+++ b/INSTALL.Windows.md
@@ -134,7 +134,7 @@ For example,
 Building the Kernel datapath module
 ---
 * We directly use the Visual Studio 2013 IDE to compile the kernel datapath.
-You can open the extensions.sln file in the IDE and build the solution.
+You can open the ovsext.sln file in the IDE and build the solution.
 
 * The kernel datapath can be compiled from command line as well.  The top
 level 'make' will invoke building the kernel datapath, if the
-- 
2.9.3.windows.1

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

Re: [ovs-dev] [ovs-dev, v2, 3/4] Unpersist data structures for address sets.

[Ryan Moats]

Ben Pfaff  wrote on 08/31/2016 02:45:36 PM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 08/31/2016 02:45 PM
> Subject: Re: [ovs-dev] [ovs-dev, v2,3/4] Unpersist data structures
> for address sets.
>
> On Wed, Aug 31, 2016 at 01:51:51PM -0500, Ryan Moats wrote:
> >
> >
> > "dev"  wrote on 08/31/2016 12:53:27 PM:
> >
> > > From: Ryan Moats/Omaha/IBM@IBMUS
> > > To: Ben Pfaff 
> > > Cc: dev@openvswitch.org
> > > Date: 08/31/2016 12:53 PM
> > > Subject: Re: [ovs-dev] [ovs-dev, v2, 3/4] Unpersist data structures
> > > for address sets.
> > > Sent by: "dev" 
> > >
> > >
> > > Ben Pfaff  wrote on 08/31/2016 12:38:34 PM:
> > >
> > > > From: Ben Pfaff 
> > > > To: Ryan Moats/Omaha/IBM@IBMUS
> > > > Cc: dev@openvswitch.org
> > > > Date: 08/31/2016 12:38 PM
> > > > Subject: Re: [ovs-dev,v2,3/4] Unpersist data structures for address
> > sets.
> > > >
> > > > On Wed, Aug 31, 2016 at 03:22:45PM +, Ryan Moats wrote:
> > > > > With the removal of incremental processing, it is no longer
> > > > > necessary to persist the data structures for storing address
> > > > > sets.  Simplify things by removing this complexity.
> > > > >
> > > > > Side effect: fixed a memory leak in expr_macros_destroy that
> > > > > is evidenced by this change.
> > > > >
> > > > > Signed-off-by: Ryan Moats 
> > > >
> > > > I think that this was still doing a lot more work than necessary.
The
> > > > "struct address_set"s were being created and destroyed but not used
for
> > > > anything in between.
> > > >
> > > > Here's my proposal.  Comments?
> > > >
> > > > --8<--cut here-->
8--
> > > >
> > > > From: Ryan Moats 
> > > > Date: Wed, 31 Aug 2016 15:22:45 +
> > > > Subject: [PATCH] Unpersist data structures for address sets.
> > > >
> > > > With the removal of incremental processing, it is no longer
> > > > necessary to persist the data structures for storing address
> > > > sets.  Simplify things by removing this complexity.
> > > >
> > > > Side effect: fixed a memory leak in expr_macros_destroy that
> > > > is evidenced by this change.
> > > >
> > > > Signed-off-by: Ryan Moats 
> > > > Signed-off-by: Ben Pfaff 
> > > > ---
> > > >  ovn/controller/lflow.c | 176 ++
> > > > +--
> > > >  ovn/lib/expr.c |   1 +
> > > >  2 files changed, 25 insertions(+), 152 deletions(-)
> > > >
> > > > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> > > > index a9c3137..efac5b3 100644
> > > > --- a/ovn/controller/lflow.c
> > > > +++ b/ovn/controller/lflow.c
> > > > @@ -38,159 +38,24 @@ VLOG_DEFINE_THIS_MODULE(lflow);
> > > >  /* Contains "struct expr_symbol"s for fields supported by OVN
lflows.
> > */
> > > >  static struct shash symtab;
> > > >
> > > > -/* Contains an internal expr datastructure that represents an
address
> > > set. */
> > > > -static struct shash expr_address_sets;
> > > > -
> > > >  void
> > > >  lflow_init(void)
> > > >  {
> > > >  ovn_init_symtab();
> > > > -shash_init(_address_sets);
> > > > -}
> > > > -
> > > > -/* Details of an address set currently in address_sets. We keep a
> > cached
> > > > - * copy of sets still in their string form here to make it easier
to
> > > compare
> > > > - * with the current values in the OVN_Southbound database. */
> > > > -struct address_set {
> > > > -char **addresses;
> > > > -size_t n_addresses;
> > > > -};
> > > > -
> > > > -/* struct address_set instances for address sets currently in the
> > > symtab,
> > > > - * hashed on the address set name. */
> > > > -static struct shash local_address_sets =
> > > > SHASH_INITIALIZER(_address_sets);
> > > > -
> > > > -static int
> > > > -addr_cmp(const void *p1, const void *p2)
> > > > -{
> > > > -const char *s1 = p1;
> > > > -const char *s2 = p2;
> > > > -return strcmp(s1, s2);
> > > > -}
> > > > -
> > > > -/* Return true if the address sets match, false otherwise. */
> > > > -static bool
> > > > -address_sets_match(const struct address_set *addr_set,
> > > > -   const struct sbrec_address_set *addr_set_rec)
> > > > -{
> > > > -char **addrs1;
> > > > -char **addrs2;
> > > > -
> > > > -if (addr_set->n_addresses != addr_set_rec->n_addresses) {
> > > > -return false;
> > > > -}
> > > > -size_t n_addresses = addr_set->n_addresses;
> > > > -
> > > > -addrs1 = xmemdup(addr_set->addresses,
> > > > - n_addresses * sizeof addr_set->addresses[0]);
> > > > -addrs2 = xmemdup(addr_set_rec->addresses,
> > > > - n_addresses * sizeof addr_set_rec->addresses
[0]);
> > > > -
> > > > -qsort(addrs1, n_addresses, sizeof *addrs1, addr_cmp);
> > > > -qsort(addrs2, n_addresses, sizeof *addrs2, addr_cmp);

Re: [ovs-dev] [PATCHv2 4/4] upcall: Replace ukeys for deleted flows.

[Jarno Rajahalme]

With a minor question below,

Acked-by: Jarno Rajahalme 

> On Aug 31, 2016, at 11:06 AM, Joe Stringer  wrote:
> 
> If a revalidator dumps/revalidates a flow during the 'dump' phase,
> resulting in the deletion of the flow, then ukey->flow_exists is set to
> false, and the ukey is kept around until the 'sweep' phase. The ukey is
> kept around to ensure that cases like duplicated dumps from the
> datapaths do not result in multiple attribution of the same stats.
> 
> However, if an upcall for this flow comes for a handler between the
> revalidator 'dump' and 'sweep' phases, the handler will lookup the ukey
> and find that the ukey exists, then skip installing a new flow entirely.
> As a result, for this period all traffic for the flow is slowpathed.
> If there is a lot of traffic hitting this flow, then it will all be
> handled in userspace until the 'sweep' phase. Eventually the
> revalidators will reach the sweep phase and delete the ukey, and
> subsequently the handlers should install a new flow.
> 
> To reduce the slowpathing of this traffic during flow table transitions,
> allow the handler to identify this case during miss upcall handling and
> replace the existing ukey with a new ukey. The handler will then be able
> to install a flow for this traffic, allowing the traffic flow to return
> to the fastpath.
> 
> Signed-off-by: Joe Stringer 
> ---
> v2: Rebase against ukey lifecycle patch.
> ---
> ofproto/ofproto-dpif-upcall.c | 32 
> 1 file changed, 32 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index ce5a392caf78..04873cc42fff 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -50,6 +50,7 @@ COVERAGE_DEFINE(dumped_duplicate_flow);
> COVERAGE_DEFINE(dumped_new_flow);
> COVERAGE_DEFINE(handler_duplicate_upcall);
> COVERAGE_DEFINE(upcall_ukey_contention);
> +COVERAGE_DEFINE(upcall_ukey_replace);
> COVERAGE_DEFINE(revalidate_missed_dp_flow);
> 
> /* A thread that reads upcalls from dpif, forwards each upcall's packet,
> @@ -1568,6 +1569,36 @@ ukey_create_from_dpif_flow(const struct udpif *udpif,
> return 0;
> }
> 
> +static bool
> +try_ukey_replace(struct umap *umap, struct udpif_key *old_ukey,
> + struct udpif_key *new_ukey)
> +OVS_REQUIRES(umap->mutex)
> +OVS_TRY_LOCK(true, new_ukey->mutex)
> +{
> +bool replaced = false;
> +
> +if (!ovs_mutex_trylock(_ukey->mutex)) {
> +if (old_ukey->state == UKEY_EVICTED) {
> +COVERAGE_INC(upcall_ukey_replace);
> +
> +/* The flow was deleted during the current revalidator dump,
> + * but its ukey won't be fully cleaned up until the sweep phase.
> + * In the mean time, we are receiving upcalls for this traffic.
> + * Expedite the (new) flow install by replacing the ukey. */
> +ovs_mutex_lock(_ukey->mutex);
> +cmap_replace(>cmap, _ukey->cmap_node,
> + _ukey->cmap_node, new_ukey->hash);
> +ovsrcu_postpone(ukey_delete__, old_ukey);
> +transition_ukey(old_ukey, UKEY_DELETED);
> +transition_ukey(new_ukey, UKEY_VISIBLE);
> +replaced = true;
> +}
> +ovs_mutex_unlock(_ukey->mutex);
> +}
> +
> +return replaced;
> +}
> +
> /* Attempts to insert a ukey into the shared ukey maps.
>  *
>  * On success, returns true, installs the ukey and returns it in a locked
> @@ -1590,6 +1621,7 @@ ukey_install__(struct udpif *udpif, struct udpif_key 
> *new_ukey)
> if (old_ukey->key_len == new_ukey->key_len
> && !memcmp(old_ukey->key, new_ukey->key, new_ukey->key_len)) {
> COVERAGE_INC(handler_duplicate_upcall);
> +locked = try_ukey_replace(umap, old_ukey, new_ukey);

Should we do the coverage only if the try_ukey_replace() returns false?

> } else {
> struct ds ds = DS_EMPTY_INITIALIZER;
> 
> -- 
> 2.9.3
> 

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

Re: [ovs-dev] [PATCHv2 3/4] upcall: Track ukey states.

[Jarno Rajahalme]

With small nits below,

Acked-by: Jarno Rajahalme 

> On Aug 31, 2016, at 11:06 AM, Joe Stringer  wrote:
> 
> Ukeys already have a well-defined lifetime that starts from being
> created, inserted into the umaps, having the corresponding flow
> installed, then the flow deleted, the ukey removed from the umap,
> rcu-deferral of its deletion, and finally freedom.
> 
> However, until now it's all been represented behind a simple boolean
> "flow_exists" with a bunch of implicit logic sprinkled around the
> accessors. This patch attempts to make the ukey lifetime a bit clearer
> by outlining the correct transitions and asserting that their lifetime
> proceeds as expected.
> 
> This should improve the readability of the current code, and also make
> the following patch easier to reason about.
> 
> Signed-off-by: Joe Stringer 
> ---
> v2: First post.
> ---
> ofproto/ofproto-dpif-upcall.c | 140 --
> 1 file changed, 94 insertions(+), 46 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index c4034f57f33e..ce5a392caf78 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -236,6 +236,17 @@ struct upcall {
> uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */
> };
> 
> +/* Ukeys must transition through these states using transition_ukey(). */
> +enum ukey_state {
> +UKEY_CREATED = 0,
> +UKEY_VISIBLE,   /* Ukey is in umap, datapath flow install is queued. 
> */
> +UKEY_OPERATIONAL,   /* Ukey is in umap, datapath flow is installed. */
> +UKEY_EVICTING,  /* Ukey is in umap, datapath flow delete is queued. 
> */
> +UKEY_EVICTED,   /* Ukey is in umap, datapath flow is deleted. */
> +UKEY_DELETED,   /* Ukey removed from umap, ukey free is deferred. */
> +};
> +#define N_UKEY_STATES (UKEY_DELETED + 1)
> +
> /* 'udpif_key's are responsible for tracking the little bit of state udpif
>  * needs to do flow expiration which can't be pulled directly from the
>  * datapath.  They may be created by any handler or revalidator thread at any
> @@ -266,8 +277,8 @@ struct udpif_key {
> long long int created OVS_GUARDED;/* Estimate of creation time. */
> uint64_t dump_seq OVS_GUARDED;/* Tracks udpif->dump_seq. */
> uint64_t reval_seq OVS_GUARDED;   /* Tracks udpif->reval_seq. */
> -bool flow_exists OVS_GUARDED; /* Ensures flows are only 
> deleted
> - once. */
> +enum ukey_state state OVS_GUARDED;/* Tracks ukey lifetime. */
> +
> /* Datapath flow actions as nlattrs.  Protected by RCU.  Read with
>  * ukey_get_actions(), and write with ukey_set_actions(). */
> OVSRCU_TYPE(struct ofpbuf *) actions;
> @@ -334,9 +345,9 @@ static int ukey_create_from_dpif_flow(const struct udpif 
> *,
>   struct udpif_key **);
> static void ukey_get_actions(struct udpif_key *, const struct nlattr 
> **actions,
>  size_t *size);
> -static bool ukey_install_start(struct udpif *, struct udpif_key *ukey);
> -static bool ukey_install_finish(struct udpif_key *ukey, int error);
> +static bool ukey_install__(struct udpif *, struct udpif_key *ukey);

You need OVS_TRY_LOCK(true, ukey->mutex) here as, and in general all 
declarations should have the same thread safety annotations as the definitions 
themselves.

> static bool ukey_install(struct udpif *udpif, struct udpif_key *ukey);
> +static void transition_ukey(struct udpif_key *, enum ukey_state dat);

You need OVS_REQUIRES(ukey->mutex) here for thread safety analysis to be done.

> static struct udpif_key *ukey_lookup(struct udpif *udpif,
>  const ovs_u128 *ufid,
>  const unsigned pmd_id);
> @@ -349,6 +360,8 @@ static enum upcall_type classify_upcall(enum 
> dpif_upcall_type type,
> 
> static void put_op_init(struct ukey_op *op, struct udpif_key *ukey,
> enum dpif_flow_put_flags flags);
> +static void delete_op_init(struct udpif *udpif, struct ukey_op *op,
> +   struct udpif_key *ukey);
> 
> static int upcall_receive(struct upcall *, const struct dpif_backer *,
>   const struct dp_packet *packet, enum 
> dpif_upcall_type,
> @@ -1337,7 +1350,7 @@ handle_upcalls(struct udpif *udpif, struct upcall 
> *upcalls,
> if (should_install_flow(udpif, upcall)) {
> struct udpif_key *ukey = upcall->ukey;
> 
> -if (ukey_install_start(udpif, ukey)) {
> +if (ukey_install(udpif, ukey)) {
> upcall->ukey_persists = true;
> put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
> }
> @@ -1359,20 +1372,23 @@ handle_upcalls(struct udpif *udpif, struct upcall 
> *upcalls,
> }
> }
> 
> -/* Execute 

Re: [ovs-dev] [PATCHv2 2/4] upcall: Only init flow_put if ukey is installed.

[Jarno Rajahalme]

With one question below,

Acked-by: Jarno Rajahalme 

> On Aug 31, 2016, at 11:06 AM, Joe Stringer  wrote:
> 
> Currently when processing a batch of upcalls, all datapath operations
> are first initialized, then later the corresponding ukeys are installed.
> If the ukey_install fails at this later point, then the code needs to
> backtrack a bit to delete the ukey and skip using the initialized
> datapath op.
> 
> It's a little simpler to only initialize the datapath operation if the
> ukey could actually be installed. The locks are held longer, but these
> locks aren't heavily contended and the extended holding of the lock will
> be removed in a subsequent patch anyway.
> 
> Signed-off-by: Joe Stringer 
> ---
> v2: First post
> ---
> ofproto/ofproto-dpif-upcall.c | 16 
> 1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index e7fcdd28c9ff..c4034f57f33e 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1337,8 +1337,10 @@ handle_upcalls(struct udpif *udpif, struct upcall 
> *upcalls,
> if (should_install_flow(udpif, upcall)) {
> struct udpif_key *ukey = upcall->ukey;
> 
> -upcall->ukey_persists = true;
> -put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
> +if (ukey_install_start(udpif, ukey)) {
> +upcall->ukey_persists = true;
> +put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
> +}
> }
> 
> if (upcall->odp_actions.size) {
> @@ -1365,16 +1367,6 @@ handle_upcalls(struct udpif *udpif, struct upcall 
> *upcalls,
>  */
> n_opsp = 0;
> for (i = 0; i < n_ops; i++) {
> -struct udpif_key *ukey = ops[i].ukey;
> -
> -if (ukey) {
> -/* If we can't install the ukey, don't install the flow. */
> -if (!ukey_install_start(udpif, ukey)) {
> -ukey_delete__(ukey);
> -ops[i].ukey = NULL;

Does the mean that we do not need to check key pointer for NULL somewhere else 
anymore?

> -continue;
> -}
> -}
> opsp[n_opsp++] = [i].dop;
> }
> dpif_operate(udpif->dpif, opsp, n_opsp);
> -- 
> 2.9.3
> 

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

Re: [ovs-dev] [PATCHv2 1/4] upcall: Reuse flow_put initializer.

[Jarno Rajahalme]

Acked-by: Jarno Rajahalme 

> On Aug 31, 2016, at 11:06 AM, Joe Stringer  wrote:
> 
> Signed-off-by: Joe Stringer 
> ---
> v2: First post
> ---
> ofproto/ofproto-dpif-upcall.c | 24 
> 1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index e4473080ad65..e7fcdd28c9ff 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -347,6 +347,9 @@ static void ukey_delete(struct umap *, struct udpif_key 
> *);
> static enum upcall_type classify_upcall(enum dpif_upcall_type type,
> const struct nlattr *userdata);
> 
> +static void put_op_init(struct ukey_op *op, struct udpif_key *ukey,
> +enum dpif_flow_put_flags flags);
> +
> static int upcall_receive(struct upcall *, const struct dpif_backer *,
>   const struct dp_packet *packet, enum 
> dpif_upcall_type,
>   const struct nlattr *userdata, const struct flow *,
> @@ -1335,19 +1338,7 @@ handle_upcalls(struct udpif *udpif, struct upcall 
> *upcalls,
> struct udpif_key *ukey = upcall->ukey;
> 
> upcall->ukey_persists = true;
> -op = [n_ops++];
> -
> -op->ukey = ukey;
> -op->dop.type = DPIF_OP_FLOW_PUT;
> -op->dop.u.flow_put.flags = DPIF_FP_CREATE;
> -op->dop.u.flow_put.key = ukey->key;
> -op->dop.u.flow_put.key_len = ukey->key_len;
> -op->dop.u.flow_put.mask = ukey->mask;
> -op->dop.u.flow_put.mask_len = ukey->mask_len;
> -op->dop.u.flow_put.ufid = upcall->ufid;
> -op->dop.u.flow_put.stats = NULL;
> -ukey_get_actions(ukey, >dop.u.flow_put.actions,
> - >dop.u.flow_put.actions_len);
> +put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
> }
> 
> if (upcall->odp_actions.size) {
> @@ -1936,11 +1927,12 @@ delete_op_init(struct udpif *udpif, struct ukey_op 
> *op, struct udpif_key *ukey)
> }
> 
> static void
> -modify_op_init(struct ukey_op *op, struct udpif_key *ukey)
> +put_op_init(struct ukey_op *op, struct udpif_key *ukey,
> +enum dpif_flow_put_flags flags)
> {
> op->ukey = ukey;
> op->dop.type = DPIF_OP_FLOW_PUT;
> -op->dop.u.flow_put.flags = DPIF_FP_MODIFY;
> +op->dop.u.flow_put.flags = flags;
> op->dop.u.flow_put.key = ukey->key;
> op->dop.u.flow_put.key_len = ukey->key_len;
> op->dop.u.flow_put.mask = ukey->mask;
> @@ -2085,7 +2077,7 @@ reval_op_init(struct ukey_op *op, enum reval_result 
> result,
> /* ukey->key_recirc_id remains, as the key is the same as before. */
> 
> ukey_set_actions(ukey, odp_actions);
> -modify_op_init(op, ukey);
> +put_op_init(op, ukey, DPIF_FP_MODIFY);
> }
> }
> 
> -- 
> 2.9.3
> 

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

Re: [ovs-dev] bug fix for miimon issue with ixgbe driver that half support mii

[Jeff Kirsher]

On Tue, 2016-08-30 at 17:35 -0400, David Hill wrote:
> Hello sir,
> 
> I do not understand either but that's what's happening.  It's a 
> fiber adapter so maybe the driver says it's supported and when trying to 
> request values from the nic itself it fails because it's no longer 
> supported... but that's only a though .
> 

One of our ixgbe maintainers has created a patch http://patchwork.ozlabs.or
g/patch/664633/ to fix the issue.  If you could have your customer please
try the patch in the above link, to verify that it fixes the issue would be
greatly appreciated.

> 
> On 08/30/2016 04:33 PM, Joe Stringer wrote:
> > 
> > On 30 August 2016 at 12:49, David Hill  wrote:
> > > 
> > > Hello sir,
> > > 
> > > We have a customer hitting an issue where ixgbe will not return
> > > an error
> > > at "SIOCGMIIPHY" but will hit it later at "SIOCGMIIREG" so instead of
> > > failling at miimon and never doing the failover, we'd like it to use
> > > ethtool
> > > which is properly returning the link state.
> > Hm, I don't understand why ixgbe would support getting the address of
> > the MII PHY in use, but not to fetch the IMM_BMSR "basic mode status
> > register"
> > from it. Is this supposed to happen? Have you tried discussing this
> > with the upstream driver developers?
> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovs-dev, v2, 3/4] Unpersist data structures for address sets.

[Ben Pfaff]

On Wed, Aug 31, 2016 at 01:51:51PM -0500, Ryan Moats wrote:
> 
> 
> "dev"  wrote on 08/31/2016 12:53:27 PM:
> 
> > From: Ryan Moats/Omaha/IBM@IBMUS
> > To: Ben Pfaff 
> > Cc: dev@openvswitch.org
> > Date: 08/31/2016 12:53 PM
> > Subject: Re: [ovs-dev] [ovs-dev, v2, 3/4] Unpersist data structures
> > for address sets.
> > Sent by: "dev" 
> >
> >
> > Ben Pfaff  wrote on 08/31/2016 12:38:34 PM:
> >
> > > From: Ben Pfaff 
> > > To: Ryan Moats/Omaha/IBM@IBMUS
> > > Cc: dev@openvswitch.org
> > > Date: 08/31/2016 12:38 PM
> > > Subject: Re: [ovs-dev,v2,3/4] Unpersist data structures for address
> sets.
> > >
> > > On Wed, Aug 31, 2016 at 03:22:45PM +, Ryan Moats wrote:
> > > > With the removal of incremental processing, it is no longer
> > > > necessary to persist the data structures for storing address
> > > > sets.  Simplify things by removing this complexity.
> > > >
> > > > Side effect: fixed a memory leak in expr_macros_destroy that
> > > > is evidenced by this change.
> > > >
> > > > Signed-off-by: Ryan Moats 
> > >
> > > I think that this was still doing a lot more work than necessary.  The
> > > "struct address_set"s were being created and destroyed but not used for
> > > anything in between.
> > >
> > > Here's my proposal.  Comments?
> > >
> > > --8<--cut here-->8--
> > >
> > > From: Ryan Moats 
> > > Date: Wed, 31 Aug 2016 15:22:45 +
> > > Subject: [PATCH] Unpersist data structures for address sets.
> > >
> > > With the removal of incremental processing, it is no longer
> > > necessary to persist the data structures for storing address
> > > sets.  Simplify things by removing this complexity.
> > >
> > > Side effect: fixed a memory leak in expr_macros_destroy that
> > > is evidenced by this change.
> > >
> > > Signed-off-by: Ryan Moats 
> > > Signed-off-by: Ben Pfaff 
> > > ---
> > >  ovn/controller/lflow.c | 176 ++
> > > +--
> > >  ovn/lib/expr.c |   1 +
> > >  2 files changed, 25 insertions(+), 152 deletions(-)
> > >
> > > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> > > index a9c3137..efac5b3 100644
> > > --- a/ovn/controller/lflow.c
> > > +++ b/ovn/controller/lflow.c
> > > @@ -38,159 +38,24 @@ VLOG_DEFINE_THIS_MODULE(lflow);
> > >  /* Contains "struct expr_symbol"s for fields supported by OVN lflows.
> */
> > >  static struct shash symtab;
> > >
> > > -/* Contains an internal expr datastructure that represents an address
> > set. */
> > > -static struct shash expr_address_sets;
> > > -
> > >  void
> > >  lflow_init(void)
> > >  {
> > >  ovn_init_symtab();
> > > -shash_init(_address_sets);
> > > -}
> > > -
> > > -/* Details of an address set currently in address_sets. We keep a
> cached
> > > - * copy of sets still in their string form here to make it easier to
> > compare
> > > - * with the current values in the OVN_Southbound database. */
> > > -struct address_set {
> > > -char **addresses;
> > > -size_t n_addresses;
> > > -};
> > > -
> > > -/* struct address_set instances for address sets currently in the
> > symtab,
> > > - * hashed on the address set name. */
> > > -static struct shash local_address_sets =
> > > SHASH_INITIALIZER(_address_sets);
> > > -
> > > -static int
> > > -addr_cmp(const void *p1, const void *p2)
> > > -{
> > > -const char *s1 = p1;
> > > -const char *s2 = p2;
> > > -return strcmp(s1, s2);
> > > -}
> > > -
> > > -/* Return true if the address sets match, false otherwise. */
> > > -static bool
> > > -address_sets_match(const struct address_set *addr_set,
> > > -   const struct sbrec_address_set *addr_set_rec)
> > > -{
> > > -char **addrs1;
> > > -char **addrs2;
> > > -
> > > -if (addr_set->n_addresses != addr_set_rec->n_addresses) {
> > > -return false;
> > > -}
> > > -size_t n_addresses = addr_set->n_addresses;
> > > -
> > > -addrs1 = xmemdup(addr_set->addresses,
> > > - n_addresses * sizeof addr_set->addresses[0]);
> > > -addrs2 = xmemdup(addr_set_rec->addresses,
> > > - n_addresses * sizeof addr_set_rec->addresses[0]);
> > > -
> > > -qsort(addrs1, n_addresses, sizeof *addrs1, addr_cmp);
> > > -qsort(addrs2, n_addresses, sizeof *addrs2, addr_cmp);
> > > -
> > > -bool res = true;
> > > -size_t i;
> > > -for (i = 0; i <  n_addresses; i++) {
> > > -if (strcmp(addrs1[i], addrs2[i])) {
> > > -res = false;
> > > -break;
> > > -}
> > > -}
> > > -
> > > -free(addrs1);
> > > -free(addrs2);
> > > -
> > > -return res;
> > >  }
> > >
> > > +/* Iterate address sets in the southbound database.  Create and update
> > the
> > > + * corresponding symtab entries as necessary. */
> > >  static void
> > > 

Re: [ovs-dev] [ovs-dev, v2, 3/4] Unpersist data structures for address sets.

[Ryan Moats]

"dev"  wrote on 08/31/2016 12:53:27 PM:

> From: Ryan Moats/Omaha/IBM@IBMUS
> To: Ben Pfaff 
> Cc: dev@openvswitch.org
> Date: 08/31/2016 12:53 PM
> Subject: Re: [ovs-dev] [ovs-dev, v2, 3/4] Unpersist data structures
> for address sets.
> Sent by: "dev" 
>
>
> Ben Pfaff  wrote on 08/31/2016 12:38:34 PM:
>
> > From: Ben Pfaff 
> > To: Ryan Moats/Omaha/IBM@IBMUS
> > Cc: dev@openvswitch.org
> > Date: 08/31/2016 12:38 PM
> > Subject: Re: [ovs-dev,v2,3/4] Unpersist data structures for address
sets.
> >
> > On Wed, Aug 31, 2016 at 03:22:45PM +, Ryan Moats wrote:
> > > With the removal of incremental processing, it is no longer
> > > necessary to persist the data structures for storing address
> > > sets.  Simplify things by removing this complexity.
> > >
> > > Side effect: fixed a memory leak in expr_macros_destroy that
> > > is evidenced by this change.
> > >
> > > Signed-off-by: Ryan Moats 
> >
> > I think that this was still doing a lot more work than necessary.  The
> > "struct address_set"s were being created and destroyed but not used for
> > anything in between.
> >
> > Here's my proposal.  Comments?
> >
> > --8<--cut here-->8--
> >
> > From: Ryan Moats 
> > Date: Wed, 31 Aug 2016 15:22:45 +
> > Subject: [PATCH] Unpersist data structures for address sets.
> >
> > With the removal of incremental processing, it is no longer
> > necessary to persist the data structures for storing address
> > sets.  Simplify things by removing this complexity.
> >
> > Side effect: fixed a memory leak in expr_macros_destroy that
> > is evidenced by this change.
> >
> > Signed-off-by: Ryan Moats 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  ovn/controller/lflow.c | 176 ++
> > +--
> >  ovn/lib/expr.c |   1 +
> >  2 files changed, 25 insertions(+), 152 deletions(-)
> >
> > diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> > index a9c3137..efac5b3 100644
> > --- a/ovn/controller/lflow.c
> > +++ b/ovn/controller/lflow.c
> > @@ -38,159 +38,24 @@ VLOG_DEFINE_THIS_MODULE(lflow);
> >  /* Contains "struct expr_symbol"s for fields supported by OVN lflows.
*/
> >  static struct shash symtab;
> >
> > -/* Contains an internal expr datastructure that represents an address
> set. */
> > -static struct shash expr_address_sets;
> > -
> >  void
> >  lflow_init(void)
> >  {
> >  ovn_init_symtab();
> > -shash_init(_address_sets);
> > -}
> > -
> > -/* Details of an address set currently in address_sets. We keep a
cached
> > - * copy of sets still in their string form here to make it easier to
> compare
> > - * with the current values in the OVN_Southbound database. */
> > -struct address_set {
> > -char **addresses;
> > -size_t n_addresses;
> > -};
> > -
> > -/* struct address_set instances for address sets currently in the
> symtab,
> > - * hashed on the address set name. */
> > -static struct shash local_address_sets =
> > SHASH_INITIALIZER(_address_sets);
> > -
> > -static int
> > -addr_cmp(const void *p1, const void *p2)
> > -{
> > -const char *s1 = p1;
> > -const char *s2 = p2;
> > -return strcmp(s1, s2);
> > -}
> > -
> > -/* Return true if the address sets match, false otherwise. */
> > -static bool
> > -address_sets_match(const struct address_set *addr_set,
> > -   const struct sbrec_address_set *addr_set_rec)
> > -{
> > -char **addrs1;
> > -char **addrs2;
> > -
> > -if (addr_set->n_addresses != addr_set_rec->n_addresses) {
> > -return false;
> > -}
> > -size_t n_addresses = addr_set->n_addresses;
> > -
> > -addrs1 = xmemdup(addr_set->addresses,
> > - n_addresses * sizeof addr_set->addresses[0]);
> > -addrs2 = xmemdup(addr_set_rec->addresses,
> > - n_addresses * sizeof addr_set_rec->addresses[0]);
> > -
> > -qsort(addrs1, n_addresses, sizeof *addrs1, addr_cmp);
> > -qsort(addrs2, n_addresses, sizeof *addrs2, addr_cmp);
> > -
> > -bool res = true;
> > -size_t i;
> > -for (i = 0; i <  n_addresses; i++) {
> > -if (strcmp(addrs1[i], addrs2[i])) {
> > -res = false;
> > -break;
> > -}
> > -}
> > -
> > -free(addrs1);
> > -free(addrs2);
> > -
> > -return res;
> >  }
> >
> > +/* Iterate address sets in the southbound database.  Create and update
> the
> > + * corresponding symtab entries as necessary. */
> >  static void
> > -address_set_destroy(struct address_set *addr_set)
> > -{
> > -size_t i;
> > -for (i = 0; i < addr_set->n_addresses; i++) {
> > -free(addr_set->addresses[i]);
> > -}
> > -if (addr_set->n_addresses) {
> > -free(addr_set->addresses);
> > -}
> > -free(addr_set);
> > -}
> > +update_address_sets(struct 

Re: [ovs-dev] [PATCH v2 1/5] ofproto-dpif-xlate: Don't unwildcard tunnel attributes on set.

[Jarno Rajahalme]

I’d put the registers and metadata field to the ‘false’ and also maybe 
non-writeable fields (ether type, frags, nw_proto, etc.) in to 
OVS_NOT_REACHED() case, just in case.

  Jarno

> On Aug 31, 2016, at 10:38 AM, Jesse Gross  wrote:
> 
> On Tue, Aug 30, 2016 at 6:47 PM, Daniele Di Proietto
>  wrote:
>> When translating a set action we also unwildcard the field in question.
>> This is done to correctly translate set actions with the value identical
>> to the ingress flow, like in the following example:
>> 
>> flow table:
>> 
>> tcp,actions=set_field:80->tcp_dst,output:5
>> 
>> ingress packet:
>> 
>> ...,tcp,tcp_dst=80
>> 
>> datapath flow
>> 
>> ...,tcp(dst=80) actions:5
>> 
>> The datapath flow must exact match the target field, because the actions
>> do not include a set field. (Otherwise a packet coming in with a
>> different tcp_dst would be matched, and its port wouldn't be altered).
>> 
>> Tunnel attributes behave differently: at the datapath layer, before
>> action processing they're cleared (we do the same at the ofproto layer
>> in xlate_actions()).  Therefore there's no need to unwildcard them,
>> because a set action would always be detected (since we zero them at the
>> beginning of xlate_ations()).
>> 
>> This fixes a problem related to the handling of Geneve options.
>> Unwildcarding non existing Geneve options (as done by a
>> set_field:X->tun_metadata on a packet coming from a non-tunnel
>> interface) would be problematic for the datapaths: the ODP translation
>> functions cannot express a match on non existing Geneve options (unlike
>> on other attributes), and the userspace datapath wouldn't be able to
>> translate them to "userspace datapath format".  In both cases the
>> installed flow would be deleted by revalidation at the first
>> opportunity.
>> 
>> Signed-off-by: Daniele Di Proietto 
> 
> I think there might be some more obscure ways of triggering this
> problem that still exist. Basically anything that can use a register
> as a target is a potential issue. For example, stack_pop, bundle, and
> multipath all look like they have the same masking behavior.
> 
> I do have a general solution in this patch (look at the bottom of
> xlate_actions() where it is adjusting the wildcards):
> http://openvswitch.org/pipermail/dev/2016-August/078484.html
> 
> I didn't realize that it was addressing an existing issue though and
> that patch certainly isn't suitable for anything other than master.
> 
> I'm also not really a big fan of the way I handled it there since it's
> a pretty coarse way to do it. I would be happy to drop it if we can
> feel comfortable that we got all of the callsites (now and in the
> future) using your method. Perhaps we can just create a helper
> function with this check builtin and then use it everywhere? That
> might be enough to be confident about the future.
> ___
> 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] [replication SMv2 7/7] ovsdb: Replication usability improvements

[Numan Siddique]

On Wed, Aug 31, 2016 at 5:36 PM, Numan Siddique  wrote:

>
>
> On Wed, Aug 31, 2016 at 12:03 AM, Andy Zhou  wrote:
>
>>
>>
>> On Tue, Aug 30, 2016 at 4:17 AM, Numan Siddique 
>> wrote:
>>
>>>
>>>
>>> On Tue, Aug 30, 2016 at 1:11 AM, Andy Zhou  wrote:
>>>


 On Mon, Aug 29, 2016 at 3:14 AM, Numan Siddique 
 wrote:

>
>
> On Sat, Aug 27, 2016 at 4:45 AM, Andy Zhou  wrote:
>
>> Added the '--no-sync' option base on feedbacks of current
>> implementation.
>>
>> Added appctl command "ovsdb-server/sync-status" based on feedbacks
>> of current implementation.
>>
>> Added a test to simulate the integration of HA manager with OVSDB
>> server using replication.
>>
>> Other documentation and API improvements.
>>
>> Signed-off-by: Andy Zhou 
>> --
>>
>> I hope to get some review comments on the command line and appctl
>> interfaces for replication. Since 2.6 is the first release of those
>> interfaces, it is easier to making changes, compare to future
>> releases.
>>
>> 
>> v1->v2: Fix creashes reported at:
>> http://openvswitch.org/pipermail/dev/2016-August/078591.html
>> ---
>>
>
> ​I haven't tested these patches yet. This patch seems to have a white
> space warning when applied.
>
 Thanks for the reported. I will fold the fix in the next version when
 posting.

 In case it helps, you can also access the patches from my private repo
 at:
   https://github.com/azhou-nicira/ovs-review/tree/ovsdb-replic
 ation-sm-v2


>>> ​
>>> Hi Andy,
>>> ​
>>> I am seeing the below crash when
>>>
>>> ​  - The ovsdb-server changes from
>>> ​master to standby and the active-ovsdb-server it is about to connect to
>>> is killed just before that or it is not reachable.
>>>
>>> ​  -
>>> ​The pacemaker OCF script calls the sync-status cmd soon after that.
>>>
>>>
>>> ​Please let me know if you need more information.​
>>>
>>>
>>> ​Core was generated by `ovsdb-server -vdbg 
>>> --log-file=/opt/stack/logs/ovsdb-server-sb.log
>>> --remote=puni'.
>>> Program terminated with signal SIGSEGV, Segmentation fault.
>>> #0  0x0041241d in replication_status () at
>>> ovsdb/replication.c:875
>>> 875SHASH_FOR_EACH (node, replication_dbs) {
>>> Missing separate debuginfos, use: dnf debuginfo-install
>>> glibc-2.23.1-10.fc24.x86_64 openssl-libs-1.0.2h-3.fc24.x86_64
>>> (gdb) bt
>>> #0  0x0041241d in replication_status () at
>>> ovsdb/replication.c:875
>>> #1  0x00406eda in ovsdb_server_get_sync_status (conn=0x1421fd0,
>>> argc=, argv=, config_=)
>>> at ovsdb/ovsdb-server.c:1480
>>> #2  0x004324ee in process_command (request=0x1421f30,
>>> conn=0x1421fd0) at lib/unixctl.c:313
>>> #3  run_connection (conn=0x1421fd0) at lib/unixctl.c:347
>>> #4  unixctl_server_run (server=server@entry=0x141e140) at
>>> lib/unixctl.c:400
>>> #5  0x00405bdc in main_loop (is_backup=0x7fff08062256,
>>> exiting=0x7fff08062257, run_process=0x0, remotes=0x7fff080622a0,
>>> unixctl=0x141e140,
>>> all_dbs=0x7fff080622e0, jsonrpc=0x13f6f00) at
>>> ovsdb/ovsdb-server.c:182
>>> #6  main (argc=, argv=) at
>>> ovsdb/ovsdb-server.c:430​
>>>
>>> Numan, thanks for the report. I think I spotted the bug:
>>
>> Currently, when replication state machine is reset,  the state update
>> takes place after a round of main loop run. this time lag
>> could lead to the back trace in case the unixctl commands was issued
>> during this time lag.  I have a fix that add another
>> state to represent the reset condition.  The fix is at:
>>
>> https://github.com/azhou-nicira/ovs-review/tree/ovsdb-replication-sm-v3
>>
>> Would you please let me know if this version works any better?. Thanks!
>>
>
> ​Sure. I would test and let you know.
>
>
​I tested the v3 version pulling from your github, and I am not seeing an
crashes.

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

Re: [ovs-dev] [PATCH 2/2] ofp-actions: Waste less memory in set field and load actions.

[Jarno Rajahalme]

> On Aug 31, 2016, at 9:55 AM, Ben Pfaff  wrote:
> 
> On Wed, Aug 31, 2016 at 08:53:20AM -0700, Jarno Rajahalme wrote:
>> Pushed to master. How about branch-2.6?
> 
> I'm inclined to put both of these on branch-2.6 as well.

Done.

  Jarno

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

Re: [ovs-dev] [PATCH] system-traffic: add a bonding test case

[Joe Stringer]

On 30 August 2016 at 13:45, Lance Richardson  wrote:
> Add a test case to check connectivity over an OVS bond, using a
> Linux bond over veth interfaces.
>
> Also added a new macro "ADD_VETH_BOND", modeled after "ADD_VETH",
> in anticipation of future additional bonding test cases.
>
> Signed-off-by: Lance Richardson 

Thanks, this passes on all relevant platforms I could test on. I
applied the following minor diff and applied this patch to master:

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 252ec8944382..1dfdcf9ea789 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -90,9 +90,9 @@ m4_define([ADD_VETH],

# ADD_VETH_BOND([ports], [namespace], [ovs-br], [bond], [mode], [ip_addr])
#
-# Add a set of veth port pairs. Ports named in the list 'ports' will
be added to
-# name space 'namespace', and the corresponding port names, prefixed by 'ovs-'
-# will be included in an OVS bond 'bond' which is added to bridge 'ovs-br'.
+# Add a set of veth port pairs. Ports named in the list 'ports' will be added
+# to 'namespace', and the corresponding port names, prefixed by 'ovs-' will
+# be included in an OVS bond 'bond' which is added to bridge 'ovs-br'.
#
# The 'bond' in 'namespace' will be brought up with static IP address
# with 'ip_addr' in CIDR notation.
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index aec5999b7d67..eaf4aba13869 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -150,8 +150,6 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i
0.3 -w 2 10.1.1.2 | FORMAT_PING
3 packets transmitted, 3 received, 0% packet loss, time 0ms
])

-ovs-appctl dpif/dump-flows br0
-
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCHv2 0/4] Replace ukeys for deleted flows

[Joe Stringer]

Recent bugs[1] have highlighted a particular situation where we may handle
significant traffic in userspace via the upcall mechanism either due to flow
table changes, or when bugs in translation logic result in unexpected deletion
of datapath flows.

The basic logic is this:
* A high-throughput flow in the datapath is revalidated near the beginning of
  the revalidation cycle ("dump" phase).
* Revalidator decides to delete the flow.
* The ukey for the flow is kept around to ensure that duplicate dumps from
  the datapath do not double-attribute (stats + side-effects).
* Once the flow is deleted, traffic begins to upcall
* During upcall processing, a flow will only be installed if the corresponding
  ukey can be set up.
* However, the old ukey is still in place until the revalidators reach the
  "sweep" phase.
* As such, the packets for this flow are handled via the upcall interface
  until the revalidator sweeps it.

When combined with a bug in translation logic, this may cause a kind of
oscillation to occur, with the packets flowing through the datapath, then up
into userspace, then back down, then back up. Under high load this could be
hundreds of milliseconds per cycle, rather than seeing packets primarily flow
through the datapath with occasional blips of userspace processing.

Even if there is no such bug, this may also have a larger impact than
necessary on high throughput flows when the flow table changes.

This series does some code cleanup then addresses the behaviour above by
detecting when this situation occurs, and attempting to replace the old
ukey with a new ukey, thereby allowing the flow to be installed, and to
expedite putting the flow back into the datapath. This should improve
OVS resiliency in some corner cases.

[1] http://openvswitch.org/pipermail/dev/2016-August/078855.html

Joe Stringer (4):
  upcall: Reuse flow_put initializer.
  upcall: Only init flow_put if ukey is installed.
  upcall: Track ukey states.
  upcall: Replace ukeys for deleted flows.

 ofproto/ofproto-dpif-upcall.c | 208 +++---
 1 file changed, 136 insertions(+), 72 deletions(-)

-- 
2.9.3

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

Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module back to full processing

[Ben Pfaff]

On Wed, Aug 31, 2016 at 12:59:05PM -0500, Ryan Moats wrote:
> Ben Pfaff  wrote on 08/31/2016 12:46:17 PM:
> 
> > From: Ben Pfaff 
> > To: Jesse Gross 
> > Cc: Ryan Moats/Omaha/IBM@IBMUS, ovs dev 
> > Date: 08/31/2016 12:46 PM
> > Subject: Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module
> > back to full processing
> >
> > On Tue, Aug 30, 2016 at 09:04:10AM -0700, Jesse Gross wrote:
> > > On Sun, Aug 28, 2016 at 3:51 PM, Ryan Moats  wrote:
> > > > diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> > > > index d99ba05..87f 100644
> > > > --- a/ovn/controller/encaps.c
> > > > +++ b/ovn/controller/encaps.c
> > > > +SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->ovnsb_idl) {
> > > > +for (int i = 0; i < chassis_rec->n_encaps; i++) {
> > > > +encap_rec = chassis_rec->encaps[i];
> > > > +
> > > > +struct encap_hash_node *encap_hash_node;
> > > > +encap_hash_node =
> > lookup_encap_uuid(_rec->header_.uuid);
> > > > +if (encap_hash_node) {
> > > > +/* A change might have invalidated our mapping.
> > Process the
> > > > + * new version and then iterate over everything
> > to see if it
> > > > + * is OK. */
> > > > +delete_encap_uuid(encap_hash_node);
> > > > +poll_immediate_wake();
> > > >  }
> > >
> > > Doesn't this result in essentially infinite wakeups? It used to be
> > > that this loop would run only when a chassis was add/removed/changed
> > > and so the if (encap_hash_node) condition would only trigger when an
> > > existing chassis is modified. However, after this change every wakeup
> > > will loop through all chassises and any existing ones will trigger
> > > another wakeup and loop, etc.
> > >
> > > In general, I don't think it makes sense to remove incremental
> > > processing without removing persistent state. The result ends up being
> > > not very logical and actually more complicated.
> >
> > I want to second Jesse's feedback here.  I think that this should really
> > be stateless, not half-incremental.
> 
> Yes that poll_immediate_wake does result in infinite wakeups and yes
> it has been removed from v2 of the patch set.
> 
> I explained the situation in [1] and that "cry for help" still holds...
> 
> [1] http://openvswitch.org/pipermail/dev/2016-August/078692.html

Ah, OK, didn't understand that, I'll try to help.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCHv2 2/4] upcall: Only init flow_put if ukey is installed.

[Joe Stringer]

Currently when processing a batch of upcalls, all datapath operations
are first initialized, then later the corresponding ukeys are installed.
If the ukey_install fails at this later point, then the code needs to
backtrack a bit to delete the ukey and skip using the initialized
datapath op.

It's a little simpler to only initialize the datapath operation if the
ukey could actually be installed. The locks are held longer, but these
locks aren't heavily contended and the extended holding of the lock will
be removed in a subsequent patch anyway.

Signed-off-by: Joe Stringer 
---
v2: First post
---
 ofproto/ofproto-dpif-upcall.c | 16 
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index e7fcdd28c9ff..c4034f57f33e 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1337,8 +1337,10 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 if (should_install_flow(udpif, upcall)) {
 struct udpif_key *ukey = upcall->ukey;
 
-upcall->ukey_persists = true;
-put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
+if (ukey_install_start(udpif, ukey)) {
+upcall->ukey_persists = true;
+put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
+}
 }
 
 if (upcall->odp_actions.size) {
@@ -1365,16 +1367,6 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
  */
 n_opsp = 0;
 for (i = 0; i < n_ops; i++) {
-struct udpif_key *ukey = ops[i].ukey;
-
-if (ukey) {
-/* If we can't install the ukey, don't install the flow. */
-if (!ukey_install_start(udpif, ukey)) {
-ukey_delete__(ukey);
-ops[i].ukey = NULL;
-continue;
-}
-}
 opsp[n_opsp++] = [i].dop;
 }
 dpif_operate(udpif->dpif, opsp, n_opsp);
-- 
2.9.3

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

[ovs-dev] [PATCHv2 4/4] upcall: Replace ukeys for deleted flows.

[Joe Stringer]

If a revalidator dumps/revalidates a flow during the 'dump' phase,
resulting in the deletion of the flow, then ukey->flow_exists is set to
false, and the ukey is kept around until the 'sweep' phase. The ukey is
kept around to ensure that cases like duplicated dumps from the
datapaths do not result in multiple attribution of the same stats.

However, if an upcall for this flow comes for a handler between the
revalidator 'dump' and 'sweep' phases, the handler will lookup the ukey
and find that the ukey exists, then skip installing a new flow entirely.
As a result, for this period all traffic for the flow is slowpathed.
If there is a lot of traffic hitting this flow, then it will all be
handled in userspace until the 'sweep' phase. Eventually the
revalidators will reach the sweep phase and delete the ukey, and
subsequently the handlers should install a new flow.

To reduce the slowpathing of this traffic during flow table transitions,
allow the handler to identify this case during miss upcall handling and
replace the existing ukey with a new ukey. The handler will then be able
to install a flow for this traffic, allowing the traffic flow to return
to the fastpath.

Signed-off-by: Joe Stringer 
---
v2: Rebase against ukey lifecycle patch.
---
 ofproto/ofproto-dpif-upcall.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index ce5a392caf78..04873cc42fff 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -50,6 +50,7 @@ COVERAGE_DEFINE(dumped_duplicate_flow);
 COVERAGE_DEFINE(dumped_new_flow);
 COVERAGE_DEFINE(handler_duplicate_upcall);
 COVERAGE_DEFINE(upcall_ukey_contention);
+COVERAGE_DEFINE(upcall_ukey_replace);
 COVERAGE_DEFINE(revalidate_missed_dp_flow);
 
 /* A thread that reads upcalls from dpif, forwards each upcall's packet,
@@ -1568,6 +1569,36 @@ ukey_create_from_dpif_flow(const struct udpif *udpif,
 return 0;
 }
 
+static bool
+try_ukey_replace(struct umap *umap, struct udpif_key *old_ukey,
+ struct udpif_key *new_ukey)
+OVS_REQUIRES(umap->mutex)
+OVS_TRY_LOCK(true, new_ukey->mutex)
+{
+bool replaced = false;
+
+if (!ovs_mutex_trylock(_ukey->mutex)) {
+if (old_ukey->state == UKEY_EVICTED) {
+COVERAGE_INC(upcall_ukey_replace);
+
+/* The flow was deleted during the current revalidator dump,
+ * but its ukey won't be fully cleaned up until the sweep phase.
+ * In the mean time, we are receiving upcalls for this traffic.
+ * Expedite the (new) flow install by replacing the ukey. */
+ovs_mutex_lock(_ukey->mutex);
+cmap_replace(>cmap, _ukey->cmap_node,
+ _ukey->cmap_node, new_ukey->hash);
+ovsrcu_postpone(ukey_delete__, old_ukey);
+transition_ukey(old_ukey, UKEY_DELETED);
+transition_ukey(new_ukey, UKEY_VISIBLE);
+replaced = true;
+}
+ovs_mutex_unlock(_ukey->mutex);
+}
+
+return replaced;
+}
+
 /* Attempts to insert a ukey into the shared ukey maps.
  *
  * On success, returns true, installs the ukey and returns it in a locked
@@ -1590,6 +1621,7 @@ ukey_install__(struct udpif *udpif, struct udpif_key 
*new_ukey)
 if (old_ukey->key_len == new_ukey->key_len
 && !memcmp(old_ukey->key, new_ukey->key, new_ukey->key_len)) {
 COVERAGE_INC(handler_duplicate_upcall);
+locked = try_ukey_replace(umap, old_ukey, new_ukey);
 } else {
 struct ds ds = DS_EMPTY_INITIALIZER;
 
-- 
2.9.3

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

[ovs-dev] [PATCHv2 1/4] upcall: Reuse flow_put initializer.

[Joe Stringer]

Signed-off-by: Joe Stringer 
---
v2: First post
---
 ofproto/ofproto-dpif-upcall.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index e4473080ad65..e7fcdd28c9ff 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -347,6 +347,9 @@ static void ukey_delete(struct umap *, struct udpif_key *);
 static enum upcall_type classify_upcall(enum dpif_upcall_type type,
 const struct nlattr *userdata);
 
+static void put_op_init(struct ukey_op *op, struct udpif_key *ukey,
+enum dpif_flow_put_flags flags);
+
 static int upcall_receive(struct upcall *, const struct dpif_backer *,
   const struct dp_packet *packet, enum 
dpif_upcall_type,
   const struct nlattr *userdata, const struct flow *,
@@ -1335,19 +1338,7 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 struct udpif_key *ukey = upcall->ukey;
 
 upcall->ukey_persists = true;
-op = [n_ops++];
-
-op->ukey = ukey;
-op->dop.type = DPIF_OP_FLOW_PUT;
-op->dop.u.flow_put.flags = DPIF_FP_CREATE;
-op->dop.u.flow_put.key = ukey->key;
-op->dop.u.flow_put.key_len = ukey->key_len;
-op->dop.u.flow_put.mask = ukey->mask;
-op->dop.u.flow_put.mask_len = ukey->mask_len;
-op->dop.u.flow_put.ufid = upcall->ufid;
-op->dop.u.flow_put.stats = NULL;
-ukey_get_actions(ukey, >dop.u.flow_put.actions,
- >dop.u.flow_put.actions_len);
+put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
 }
 
 if (upcall->odp_actions.size) {
@@ -1936,11 +1927,12 @@ delete_op_init(struct udpif *udpif, struct ukey_op *op, 
struct udpif_key *ukey)
 }
 
 static void
-modify_op_init(struct ukey_op *op, struct udpif_key *ukey)
+put_op_init(struct ukey_op *op, struct udpif_key *ukey,
+enum dpif_flow_put_flags flags)
 {
 op->ukey = ukey;
 op->dop.type = DPIF_OP_FLOW_PUT;
-op->dop.u.flow_put.flags = DPIF_FP_MODIFY;
+op->dop.u.flow_put.flags = flags;
 op->dop.u.flow_put.key = ukey->key;
 op->dop.u.flow_put.key_len = ukey->key_len;
 op->dop.u.flow_put.mask = ukey->mask;
@@ -2085,7 +2077,7 @@ reval_op_init(struct ukey_op *op, enum reval_result 
result,
 /* ukey->key_recirc_id remains, as the key is the same as before. */
 
 ukey_set_actions(ukey, odp_actions);
-modify_op_init(op, ukey);
+put_op_init(op, ukey, DPIF_FP_MODIFY);
 }
 }
 
-- 
2.9.3

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

[ovs-dev] [PATCHv2 3/4] upcall: Track ukey states.

[Joe Stringer]

Ukeys already have a well-defined lifetime that starts from being
created, inserted into the umaps, having the corresponding flow
installed, then the flow deleted, the ukey removed from the umap,
rcu-deferral of its deletion, and finally freedom.

However, until now it's all been represented behind a simple boolean
"flow_exists" with a bunch of implicit logic sprinkled around the
accessors. This patch attempts to make the ukey lifetime a bit clearer
by outlining the correct transitions and asserting that their lifetime
proceeds as expected.

This should improve the readability of the current code, and also make
the following patch easier to reason about.

Signed-off-by: Joe Stringer 
---
v2: First post.
---
 ofproto/ofproto-dpif-upcall.c | 140 --
 1 file changed, 94 insertions(+), 46 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index c4034f57f33e..ce5a392caf78 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -236,6 +236,17 @@ struct upcall {
 uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */
 };
 
+/* Ukeys must transition through these states using transition_ukey(). */
+enum ukey_state {
+UKEY_CREATED = 0,
+UKEY_VISIBLE,   /* Ukey is in umap, datapath flow install is queued. */
+UKEY_OPERATIONAL,   /* Ukey is in umap, datapath flow is installed. */
+UKEY_EVICTING,  /* Ukey is in umap, datapath flow delete is queued. */
+UKEY_EVICTED,   /* Ukey is in umap, datapath flow is deleted. */
+UKEY_DELETED,   /* Ukey removed from umap, ukey free is deferred. */
+};
+#define N_UKEY_STATES (UKEY_DELETED + 1)
+
 /* 'udpif_key's are responsible for tracking the little bit of state udpif
  * needs to do flow expiration which can't be pulled directly from the
  * datapath.  They may be created by any handler or revalidator thread at any
@@ -266,8 +277,8 @@ struct udpif_key {
 long long int created OVS_GUARDED;/* Estimate of creation time. */
 uint64_t dump_seq OVS_GUARDED;/* Tracks udpif->dump_seq. */
 uint64_t reval_seq OVS_GUARDED;   /* Tracks udpif->reval_seq. */
-bool flow_exists OVS_GUARDED; /* Ensures flows are only deleted
- once. */
+enum ukey_state state OVS_GUARDED;/* Tracks ukey lifetime. */
+
 /* Datapath flow actions as nlattrs.  Protected by RCU.  Read with
  * ukey_get_actions(), and write with ukey_set_actions(). */
 OVSRCU_TYPE(struct ofpbuf *) actions;
@@ -334,9 +345,9 @@ static int ukey_create_from_dpif_flow(const struct udpif *,
   struct udpif_key **);
 static void ukey_get_actions(struct udpif_key *, const struct nlattr **actions,
  size_t *size);
-static bool ukey_install_start(struct udpif *, struct udpif_key *ukey);
-static bool ukey_install_finish(struct udpif_key *ukey, int error);
+static bool ukey_install__(struct udpif *, struct udpif_key *ukey);
 static bool ukey_install(struct udpif *udpif, struct udpif_key *ukey);
+static void transition_ukey(struct udpif_key *, enum ukey_state dst);
 static struct udpif_key *ukey_lookup(struct udpif *udpif,
  const ovs_u128 *ufid,
  const unsigned pmd_id);
@@ -349,6 +360,8 @@ static enum upcall_type classify_upcall(enum 
dpif_upcall_type type,
 
 static void put_op_init(struct ukey_op *op, struct udpif_key *ukey,
 enum dpif_flow_put_flags flags);
+static void delete_op_init(struct udpif *udpif, struct ukey_op *op,
+   struct udpif_key *ukey);
 
 static int upcall_receive(struct upcall *, const struct dpif_backer *,
   const struct dp_packet *packet, enum 
dpif_upcall_type,
@@ -1337,7 +1350,7 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 if (should_install_flow(udpif, upcall)) {
 struct udpif_key *ukey = upcall->ukey;
 
-if (ukey_install_start(udpif, ukey)) {
+if (ukey_install(udpif, ukey)) {
 upcall->ukey_persists = true;
 put_op_init([n_ops++], ukey, DPIF_FP_CREATE);
 }
@@ -1359,20 +1372,23 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 }
 }
 
-/* Execute batch.
- *
- * We install ukeys before installing the flows, locking them for exclusive
- * access by this thread for the period of installation. This ensures that
- * other threads won't attempt to delete the flows as we are creating them.
- */
+/* Execute batch. */
 n_opsp = 0;
 for (i = 0; i < n_ops; i++) {
 opsp[n_opsp++] = [i].dop;
 }
 dpif_operate(udpif->dpif, opsp, n_opsp);
 for (i = 0; i < n_ops; i++) {
-if (ops[i].ukey) {
-ukey_install_finish(ops[i].ukey, 

Re: [ovs-dev] [PATCH V2 02/10] python tests: Skip python tests that kill the python daemon

[Guru Shetty]

On 31 August 2016 at 00:45, Paul Boca  wrote:

> Hi Guru,
>
>
>
> There are other tests that check if the daemon is running fine.
>
> In my opinion we could skip this test on Windows and let it on Linux to
> run.
>

All right. I applied this.

>
>
> Paul
>
>
>
> *From:* Guru Shetty [mailto:g...@ovn.org]
> *Sent:* Tuesday, August 30, 2016 6:18 PM
> *To:* Paul Boca
> *Cc:* dev@openvswitch.org
> *Subject:* Re: [ovs-dev] [PATCH V2 02/10] python tests: Skip python tests
> that kill the python daemon
>
>
>
>
>
>
>
> On 30 August 2016 at 05:00, Paul Boca 
> wrote:
>
> If the python script is killed with `kill` command, the atexit
> handler doesn't gets executed on Windows.
> The kill of the process is done using NtTerminateProcess which
> doesn't sends a signal to the process itself, if just terminates the
> process
> from kernel mode.
>
> Signed-off-by: Paul-Daniel Boca 
>
>
>
> Instead of skipping the test, why not just skip the line that tests for
> pid after the process is killed. I imagine that the test is still useful in
> other respects, like whether detach works fine?
>
>
>
> ---
> V2: Initial commit
> ---
>  tests/daemon-py.at | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tests/daemon-py.at b/tests/daemon-py.at
> index 96dea07..11833c8 100644
> --- a/tests/daemon-py.at
> +++ b/tests/daemon-py.at
> @@ -126,6 +126,8 @@ DAEMON_MONITOR_RESTART_PYN([Python3],
> [$HAVE_PYTHON3], [$PYTHON3])
>  m4_define([DAEMON_DETACH_PYN],
>[AT_SETUP([daemon --detach - $1])
> AT_SKIP_IF([test $2 = no])
> +   # Skip this test for Windows, the pid file not removed if the daemon
> is killed
> +   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
> AT_CAPTURE_FILE([pid])
> # Start the daemon and make sure that the pidfile exists immediately.
> # We don't wait for the pidfile to get created because the daemon is
> --
> 2.7.2.windows.1
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module back to full processing

[Ryan Moats]

Ben Pfaff  wrote on 08/31/2016 12:46:17 PM:

> From: Ben Pfaff 
> To: Jesse Gross 
> Cc: Ryan Moats/Omaha/IBM@IBMUS, ovs dev 
> Date: 08/31/2016 12:46 PM
> Subject: Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module
> back to full processing
>
> On Tue, Aug 30, 2016 at 09:04:10AM -0700, Jesse Gross wrote:
> > On Sun, Aug 28, 2016 at 3:51 PM, Ryan Moats  wrote:
> > > diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> > > index d99ba05..87f 100644
> > > --- a/ovn/controller/encaps.c
> > > +++ b/ovn/controller/encaps.c
> > > +SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->ovnsb_idl) {
> > > +for (int i = 0; i < chassis_rec->n_encaps; i++) {
> > > +encap_rec = chassis_rec->encaps[i];
> > > +
> > > +struct encap_hash_node *encap_hash_node;
> > > +encap_hash_node =
> lookup_encap_uuid(_rec->header_.uuid);
> > > +if (encap_hash_node) {
> > > +/* A change might have invalidated our mapping.
> Process the
> > > + * new version and then iterate over everything
> to see if it
> > > + * is OK. */
> > > +delete_encap_uuid(encap_hash_node);
> > > +poll_immediate_wake();
> > >  }
> >
> > Doesn't this result in essentially infinite wakeups? It used to be
> > that this loop would run only when a chassis was add/removed/changed
> > and so the if (encap_hash_node) condition would only trigger when an
> > existing chassis is modified. However, after this change every wakeup
> > will loop through all chassises and any existing ones will trigger
> > another wakeup and loop, etc.
> >
> > In general, I don't think it makes sense to remove incremental
> > processing without removing persistent state. The result ends up being
> > not very logical and actually more complicated.
>
> I want to second Jesse's feedback here.  I think that this should really
> be stateless, not half-incremental.

Yes that poll_immediate_wake does result in infinite wakeups and yes
it has been removed from v2 of the patch set.

I explained the situation in [1] and that "cry for help" still holds...

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

Re: [ovs-dev] [ovs-dev, v2, 3/4] Unpersist data structures for address sets.

[Ryan Moats]

Ben Pfaff  wrote on 08/31/2016 12:38:34 PM:

> From: Ben Pfaff 
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: dev@openvswitch.org
> Date: 08/31/2016 12:38 PM
> Subject: Re: [ovs-dev,v2,3/4] Unpersist data structures for address sets.
>
> On Wed, Aug 31, 2016 at 03:22:45PM +, Ryan Moats wrote:
> > With the removal of incremental processing, it is no longer
> > necessary to persist the data structures for storing address
> > sets.  Simplify things by removing this complexity.
> >
> > Side effect: fixed a memory leak in expr_macros_destroy that
> > is evidenced by this change.
> >
> > Signed-off-by: Ryan Moats 
>
> I think that this was still doing a lot more work than necessary.  The
> "struct address_set"s were being created and destroyed but not used for
> anything in between.
>
> Here's my proposal.  Comments?
>
> --8<--cut here-->8--
>
> From: Ryan Moats 
> Date: Wed, 31 Aug 2016 15:22:45 +
> Subject: [PATCH] Unpersist data structures for address sets.
>
> With the removal of incremental processing, it is no longer
> necessary to persist the data structures for storing address
> sets.  Simplify things by removing this complexity.
>
> Side effect: fixed a memory leak in expr_macros_destroy that
> is evidenced by this change.
>
> Signed-off-by: Ryan Moats 
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/controller/lflow.c | 176 ++
> +--
>  ovn/lib/expr.c |   1 +
>  2 files changed, 25 insertions(+), 152 deletions(-)
>
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index a9c3137..efac5b3 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -38,159 +38,24 @@ VLOG_DEFINE_THIS_MODULE(lflow);
>  /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
>  static struct shash symtab;
>
> -/* Contains an internal expr datastructure that represents an address
set. */
> -static struct shash expr_address_sets;
> -
>  void
>  lflow_init(void)
>  {
>  ovn_init_symtab();
> -shash_init(_address_sets);
> -}
> -
> -/* Details of an address set currently in address_sets. We keep a cached
> - * copy of sets still in their string form here to make it easier to
compare
> - * with the current values in the OVN_Southbound database. */
> -struct address_set {
> -char **addresses;
> -size_t n_addresses;
> -};
> -
> -/* struct address_set instances for address sets currently in the
symtab,
> - * hashed on the address set name. */
> -static struct shash local_address_sets =
> SHASH_INITIALIZER(_address_sets);
> -
> -static int
> -addr_cmp(const void *p1, const void *p2)
> -{
> -const char *s1 = p1;
> -const char *s2 = p2;
> -return strcmp(s1, s2);
> -}
> -
> -/* Return true if the address sets match, false otherwise. */
> -static bool
> -address_sets_match(const struct address_set *addr_set,
> -   const struct sbrec_address_set *addr_set_rec)
> -{
> -char **addrs1;
> -char **addrs2;
> -
> -if (addr_set->n_addresses != addr_set_rec->n_addresses) {
> -return false;
> -}
> -size_t n_addresses = addr_set->n_addresses;
> -
> -addrs1 = xmemdup(addr_set->addresses,
> - n_addresses * sizeof addr_set->addresses[0]);
> -addrs2 = xmemdup(addr_set_rec->addresses,
> - n_addresses * sizeof addr_set_rec->addresses[0]);
> -
> -qsort(addrs1, n_addresses, sizeof *addrs1, addr_cmp);
> -qsort(addrs2, n_addresses, sizeof *addrs2, addr_cmp);
> -
> -bool res = true;
> -size_t i;
> -for (i = 0; i <  n_addresses; i++) {
> -if (strcmp(addrs1[i], addrs2[i])) {
> -res = false;
> -break;
> -}
> -}
> -
> -free(addrs1);
> -free(addrs2);
> -
> -return res;
>  }
>
> +/* Iterate address sets in the southbound database.  Create and update
the
> + * corresponding symtab entries as necessary. */
>  static void
> -address_set_destroy(struct address_set *addr_set)
> -{
> -size_t i;
> -for (i = 0; i < addr_set->n_addresses; i++) {
> -free(addr_set->addresses[i]);
> -}
> -if (addr_set->n_addresses) {
> -free(addr_set->addresses);
> -}
> -free(addr_set);
> -}
> +update_address_sets(struct controller_ctx *ctx,
> +struct shash *expr_address_sets_p)
>
> -static void
> -update_address_sets(struct controller_ctx *ctx)
>  {
> -/* Remember the names of all address sets currently in
expr_address_sets
> - * so we can detect address sets that have been deleted. */
> -struct sset cur_addr_set_names = SSET_INITIALIZER
(_addr_set_names);
> -
> -struct shash_node *node;
> -SHASH_FOR_EACH (node, _address_sets) {
> -sset_add(_addr_set_names, node->name);
> +const struct sbrec_address_set *as;
> +SBREC_ADDRESS_SET_FOR_EACH (as, ctx->ovnsb_idl) {
> +

Re: [ovs-dev] [PATCH] datapath-windows: remove invalid ASSERT in Flow.c

[Sairam Venugopal]

Thanks for fixing this.

Acked-by: Sairam Venugopal 


On 8/31/16, 3:33 AM, "Nithin Raju"  wrote:

>Since the Geneve changes, the key->l2.offset will no longer be 0 when
>the tunnel key is valid within the OVS flow key. key->l2.offset would
>be determined by the amount of tunnel options.
>
>Signed-off-by: Nithin Raju 
>---
> datapath-windows/ovsext/DpInternal.h | 9 ++---
> datapath-windows/ovsext/Flow.c   | 1 -
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
>diff --git a/datapath-windows/ovsext/DpInternal.h
>b/datapath-windows/ovsext/DpInternal.h
>index 22599a0..f62fc55 100644
>--- a/datapath-windows/ovsext/DpInternal.h
>+++ b/datapath-windows/ovsext/DpInternal.h
>@@ -157,17 +157,20 @@ typedef union OvsIPv4TunnelKey {
> uint64_t attr[NUM_PKT_ATTR_REQUIRED];
> } OvsIPv4TunnelKey; /* Size of 280 byte. */
> 
>-__inline uint8_t TunnelKeyGetOptionsOffset(const OvsIPv4TunnelKey *key)
>+static __inline uint8_t
>+TunnelKeyGetOptionsOffset(const OvsIPv4TunnelKey *key)
> {
> return TUN_OPT_MAX_LEN - key->tunOptLen;
> }
> 
>-__inline uint8_t* TunnelKeyGetOptions(OvsIPv4TunnelKey *key)
>+static __inline uint8_t *
>+TunnelKeyGetOptions(OvsIPv4TunnelKey *key)
> {
> return key->tunOpts + TunnelKeyGetOptionsOffset(key);
> }
> 
>-__inline uint16_t TunnelKeyGetRealSize(OvsIPv4TunnelKey *key)
>+static __inline uint16_t
>+TunnelKeyGetRealSize(OvsIPv4TunnelKey *key)
> {
> return sizeof(OvsIPv4TunnelKey) - TunnelKeyGetOptionsOffset(key);
> }
>diff --git a/datapath-windows/ovsext/Flow.c
>b/datapath-windows/ovsext/Flow.c
>index 7a57f96..439fb28 100644
>--- a/datapath-windows/ovsext/Flow.c
>+++ b/datapath-windows/ovsext/Flow.c
>@@ -2595,7 +2595,6 @@ OvsHashFlow(const OvsFlowKey *key)
> UINT8 *start;
> 
> ASSERT(key->tunKey.dst || offset == sizeof(OvsIPv4TunnelKey));
>-ASSERT(!key->tunKey.dst || offset == 0);
> start = (UINT8 *)key + offset;
> return OvsJhashBytes(start, size, 0);
> }
>-- 
>2.6.2
>
>___
>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=Dc
>ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ=p1gnLO7zJl1KBtRVs49sfBR16DmHeu
>51x1Qy2-9OKYI=Kng2MYVCWXbv7I1RCRQqZq7gSyN9CsO1bJC04pjVOZA= 

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

Re: [ovs-dev] [PATCH] ovn-controller: Convert encaps module back to full processing

[Ben Pfaff]

On Tue, Aug 30, 2016 at 09:04:10AM -0700, Jesse Gross wrote:
> On Sun, Aug 28, 2016 at 3:51 PM, Ryan Moats  wrote:
> > diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> > index d99ba05..87f 100644
> > --- a/ovn/controller/encaps.c
> > +++ b/ovn/controller/encaps.c
> > +SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->ovnsb_idl) {
> > +for (int i = 0; i < chassis_rec->n_encaps; i++) {
> > +encap_rec = chassis_rec->encaps[i];
> > +
> > +struct encap_hash_node *encap_hash_node;
> > +encap_hash_node = lookup_encap_uuid(_rec->header_.uuid);
> > +if (encap_hash_node) {
> > +/* A change might have invalidated our mapping. Process the
> > + * new version and then iterate over everything to see if 
> > it
> > + * is OK. */
> > +delete_encap_uuid(encap_hash_node);
> > +poll_immediate_wake();
> >  }
> 
> Doesn't this result in essentially infinite wakeups? It used to be
> that this loop would run only when a chassis was add/removed/changed
> and so the if (encap_hash_node) condition would only trigger when an
> existing chassis is modified. However, after this change every wakeup
> will loop through all chassises and any existing ones will trigger
> another wakeup and loop, etc.
> 
> In general, I don't think it makes sense to remove incremental
> processing without removing persistent state. The result ends up being
> not very logical and actually more complicated.

I want to second Jesse's feedback here.  I think that this should really
be stateless, not half-incremental.

Thanks,

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

Re: [ovs-dev] [PATCH v2 1/5] ofproto-dpif-xlate: Don't unwildcard tunnel attributes on set.

[Jesse Gross]

On Tue, Aug 30, 2016 at 6:47 PM, Daniele Di Proietto
 wrote:
> When translating a set action we also unwildcard the field in question.
> This is done to correctly translate set actions with the value identical
> to the ingress flow, like in the following example:
>
> flow table:
>
> tcp,actions=set_field:80->tcp_dst,output:5
>
> ingress packet:
>
> ...,tcp,tcp_dst=80
>
> datapath flow
>
> ...,tcp(dst=80) actions:5
>
> The datapath flow must exact match the target field, because the actions
> do not include a set field. (Otherwise a packet coming in with a
> different tcp_dst would be matched, and its port wouldn't be altered).
>
> Tunnel attributes behave differently: at the datapath layer, before
> action processing they're cleared (we do the same at the ofproto layer
> in xlate_actions()).  Therefore there's no need to unwildcard them,
> because a set action would always be detected (since we zero them at the
> beginning of xlate_ations()).
>
> This fixes a problem related to the handling of Geneve options.
> Unwildcarding non existing Geneve options (as done by a
> set_field:X->tun_metadata on a packet coming from a non-tunnel
> interface) would be problematic for the datapaths: the ODP translation
> functions cannot express a match on non existing Geneve options (unlike
> on other attributes), and the userspace datapath wouldn't be able to
> translate them to "userspace datapath format".  In both cases the
> installed flow would be deleted by revalidation at the first
> opportunity.
>
> Signed-off-by: Daniele Di Proietto 

I think there might be some more obscure ways of triggering this
problem that still exist. Basically anything that can use a register
as a target is a potential issue. For example, stack_pop, bundle, and
multipath all look like they have the same masking behavior.

I do have a general solution in this patch (look at the bottom of
xlate_actions() where it is adjusting the wildcards):
http://openvswitch.org/pipermail/dev/2016-August/078484.html

I didn't realize that it was addressing an existing issue though and
that patch certainly isn't suitable for anything other than master.

I'm also not really a big fan of the way I handled it there since it's
a pretty coarse way to do it. I would be happy to drop it if we can
feel comfortable that we got all of the callsites (now and in the
future) using your method. Perhaps we can just create a helper
function with this check builtin and then use it everywhere? That
might be enough to be confident about the future.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovs-dev, v2, 3/4] Unpersist data structures for address sets.

[Ben Pfaff]

On Wed, Aug 31, 2016 at 03:22:45PM +, Ryan Moats wrote:
> With the removal of incremental processing, it is no longer
> necessary to persist the data structures for storing address
> sets.  Simplify things by removing this complexity.
> 
> Side effect: fixed a memory leak in expr_macros_destroy that
> is evidenced by this change.
> 
> Signed-off-by: Ryan Moats 

I think that this was still doing a lot more work than necessary.  The
"struct address_set"s were being created and destroyed but not used for
anything in between.

Here's my proposal.  Comments?

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

From: Ryan Moats 
Date: Wed, 31 Aug 2016 15:22:45 +
Subject: [PATCH] Unpersist data structures for address sets.

With the removal of incremental processing, it is no longer
necessary to persist the data structures for storing address
sets.  Simplify things by removing this complexity.

Side effect: fixed a memory leak in expr_macros_destroy that
is evidenced by this change.

Signed-off-by: Ryan Moats 
Signed-off-by: Ben Pfaff 
---
 ovn/controller/lflow.c | 176 +++--
 ovn/lib/expr.c |   1 +
 2 files changed, 25 insertions(+), 152 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index a9c3137..efac5b3 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -38,159 +38,24 @@ VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
-/* Contains an internal expr datastructure that represents an address set. */
-static struct shash expr_address_sets;
-
 void
 lflow_init(void)
 {
 ovn_init_symtab();
-shash_init(_address_sets);
-}
-
-/* Details of an address set currently in address_sets. We keep a cached
- * copy of sets still in their string form here to make it easier to compare
- * with the current values in the OVN_Southbound database. */
-struct address_set {
-char **addresses;
-size_t n_addresses;
-};
-
-/* struct address_set instances for address sets currently in the symtab,
- * hashed on the address set name. */
-static struct shash local_address_sets = 
SHASH_INITIALIZER(_address_sets);
-
-static int
-addr_cmp(const void *p1, const void *p2)
-{
-const char *s1 = p1;
-const char *s2 = p2;
-return strcmp(s1, s2);
-}
-
-/* Return true if the address sets match, false otherwise. */
-static bool
-address_sets_match(const struct address_set *addr_set,
-   const struct sbrec_address_set *addr_set_rec)
-{
-char **addrs1;
-char **addrs2;
-
-if (addr_set->n_addresses != addr_set_rec->n_addresses) {
-return false;
-}
-size_t n_addresses = addr_set->n_addresses;
-
-addrs1 = xmemdup(addr_set->addresses,
- n_addresses * sizeof addr_set->addresses[0]);
-addrs2 = xmemdup(addr_set_rec->addresses,
- n_addresses * sizeof addr_set_rec->addresses[0]);
-
-qsort(addrs1, n_addresses, sizeof *addrs1, addr_cmp);
-qsort(addrs2, n_addresses, sizeof *addrs2, addr_cmp);
-
-bool res = true;
-size_t i;
-for (i = 0; i <  n_addresses; i++) {
-if (strcmp(addrs1[i], addrs2[i])) {
-res = false;
-break;
-}
-}
-
-free(addrs1);
-free(addrs2);
-
-return res;
 }
 
+/* Iterate address sets in the southbound database.  Create and update the
+ * corresponding symtab entries as necessary. */
 static void
-address_set_destroy(struct address_set *addr_set)
-{
-size_t i;
-for (i = 0; i < addr_set->n_addresses; i++) {
-free(addr_set->addresses[i]);
-}
-if (addr_set->n_addresses) {
-free(addr_set->addresses);
-}
-free(addr_set);
-}
+update_address_sets(struct controller_ctx *ctx,
+struct shash *expr_address_sets_p)
 
-static void
-update_address_sets(struct controller_ctx *ctx)
 {
-/* Remember the names of all address sets currently in expr_address_sets
- * so we can detect address sets that have been deleted. */
-struct sset cur_addr_set_names = SSET_INITIALIZER(_addr_set_names);
-
-struct shash_node *node;
-SHASH_FOR_EACH (node, _address_sets) {
-sset_add(_addr_set_names, node->name);
+const struct sbrec_address_set *as;
+SBREC_ADDRESS_SET_FOR_EACH (as, ctx->ovnsb_idl) {
+expr_macros_add(expr_address_sets_p, as->name,
+(const char *const *) as->addresses, as->n_addresses);
 }
-
-/* Iterate address sets in the southbound database.  Create and update the
- * corresponding symtab entries as necessary. */
-const struct sbrec_address_set *addr_set_rec;
-SBREC_ADDRESS_SET_FOR_EACH (addr_set_rec, ctx->ovnsb_idl) {
-struct address_set *addr_set =
-shash_find_data(_address_sets, addr_set_rec->name);
-
-bool create_set = false;
-  

Re: [ovs-dev] [ovs-dev, v2, 1/4] ovn-controller: Back out incremental processing

[Ben Pfaff]

On Wed, Aug 31, 2016 at 03:22:43PM +, Ryan Moats wrote:
> As [1] indicates, incremental processing hasn't resulted
> in an improvement worth the complexity it has added.
> This patch backs out all of the code specific to incremental
> processing, along with the persisting of OF flows,
> logical ports, multicast groups, all_lports, local and patched
> datapaths.
> 
> Persisted objects in the ovn/controller/physical.c module will
> be used by a future patch set to determine if physical changes
> have occurred.
> 
> Future patch sets in the series will convert
> the ovn/controller/encaps.c module back to full processing
> and remove the persistance of address sets in the
> ovn/controller/lflow.c module.
> 
> [1] http://openvswitch.org/pipermail/dev/2016-August/078272.html
> 
> Signed-off-by: Ryan Moats 

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

Re: [ovs-dev] [PATCH v2] ovs-monitor-ipsec: Fix flake8-check semicolon error

[Ben Pfaff]

On Wed, Aug 31, 2016 at 09:33:24AM -0700, Amitabha Biswas wrote:
> Fixes: 19cd0a87
> 
> Signed-off-by: Amitabha Biswas 
> Acked-by: Numan Siddique 

Thanks, applied to master.

I adjusted the commit message to match our usual style:

ovs-monitor-ipsec: Fix Python style.

Found by flake8.

Fixes: 19cd0a87827e ("ipsec: Do not allow ipsec_gre tunnel traffic to exit 
unencrypted")
Signed-off-by: Amitabha Biswas 
Acked-by: Numan Siddique 
Signed-off-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 2/2] ofp-actions: Waste less memory in set field and load actions.

[Ben Pfaff]

On Wed, Aug 31, 2016 at 08:53:20AM -0700, Jarno Rajahalme wrote:
> Pushed to master. How about branch-2.6?

I'm inclined to put both of these on branch-2.6 as well.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] one issue about vhost xstats with/without CRC

[Jesse Gross]

On Wed, Aug 31, 2016 at 2:30 AM, Yang, Zhiyong  wrote:
> Hi, all:
>
> Physical NIC has a set of hardware counters, such as
> u64 prc64;
> u64 prc127;
> u64 prc255; etc.
> DPDK counts the prc64 in two ways. Physical NIC counts prc64 with CRC by
> hardware. Virtio computes the counter like prc64 without CRC. This will cause
> the conflict, when a 64 packet from outer network is sent to VM(virtio), NIC
> will show prc64 + 1, virtio will actually receive the 64-4(CRC) = 60 bytes 
> pkt,
> undersize(<64) counter will be increased, since the Length of the packet will
> be subtracted CRC by hardware(NIC offload enable) or software((NIC offload
> disable)).
>
> if vhost xstats implements like NIC, It will solve the  consistency issue. 
> But when
> running vm to vm or virtio/vhost loopback test, xstats counters will count
> non-existent CRC bytes .
>
> How should Packets length of virtio/vhost be counted from OVS perspective?
> With or without CRC?

All other stats in OVS (including flows and other counters that don't
come from hardware) count bytes without the CRC. Presumably it would
be best to adjust the physical NIC stats with DPDK to do the same.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v2] ovs-monitor-ipsec: Fix flake8-check semicolon error

[Amitabha Biswas]

Fixes: 19cd0a87

Signed-off-by: Amitabha Biswas 
Acked-by: Numan Siddique 
---
 debian/ovs-monitor-ipsec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
index e6617b0..6bc26aa 100755
--- a/debian/ovs-monitor-ipsec
+++ b/debian/ovs-monitor-ipsec
@@ -333,7 +333,7 @@ class IPsec(object):
 self.call_ip_xfrm(["policy", "add", "src", "0.0.0.0/0", "dst",
"0.0.0.0/0", "proto", "gre", "dir", "out",
"mark", IPSEC_MARK, "mask", IPSEC_MARK,
-   "action", "block", "priority", "4294967295"]);
+   "action", "block", "priority", "4294967295"])
 
 def spd_add(self, local_ip, remote_ip):
 cmds = ("spdadd %s %s gre -P out ipsec esp/transport//require;\n" %
-- 
2.7.4 (Apple Git-66)

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

Re: [ovs-dev] [PATCH 1/2] ofp-actions: Waste less memory in learn actions.

[Jarno Rajahalme]

> On Aug 30, 2016, at 8:55 AM, Ben Pfaff  wrote:
> 
> On Tue, Aug 23, 2016 at 05:51:36PM -0700, Jarno Rajahalme wrote:
>> Make the immediate data member 'src_imm' of a learn spec allocated at
>> the end of the action for just the right size.  This, together with
>> some structure packing saves on average of ~128 bytes for each learn
>> spec in each learn action.  Typical learn actions have about 4 specs
>> each, so this amounts to saving about 0.5kb for each learn action.
>> 
>> Signed-off-by: Jarno Rajahalme 
> 
> Acked-by: Ben Pfaff 

Thanks for the review, pushed to master. Should I backport this to branch-2.6 
as well?

  Jarno

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

Re: [ovs-dev] [PATCH 2/2] ofp-actions: Waste less memory in set field and load actions.

[Jarno Rajahalme]

> On Aug 30, 2016, at 9:06 AM, Ben Pfaff  wrote:
> 
> On Tue, Aug 23, 2016 at 05:51:37PM -0700, Jarno Rajahalme wrote:
>> Change the value and mask to be added to the end of the set field
>> action without any extra bytes, exept for the usual ofp-actions
>> padding to 8 bytes.  Together with some structure member packing this
>> saves on average about to 256 bytes for each set field and load action
>> (set field internal representation is also used for load actions).
>> 
>> On a specific production data set each flow entry uses on average
>> about 4.1 load and set field actions.  This means that with this patch
>> an average of more than 1kb can be saved for each flow with such a
>> flow table.
>> 
>> Signed-off-by: Jarno Rajahalme 
> 
> Thanks for working on this!
> 
> Building on 32-bit I get assertion failures without:
> 
> diff --git a/include/openvswitch/ofp-actions.h 
> b/include/openvswitch/ofp-actions.h
> index 9eaa78c..198eedd 100644
> --- a/include/openvswitch/ofp-actions.h
> +++ b/include/openvswitch/ofp-actions.h
> @@ -479,9 +479,11 @@ struct ofpact_stack {
>  *
>  * Used for NXAST_REG_LOAD and OFPAT12_SET_FIELD. */
> struct ofpact_set_field {
> -struct ofpact ofpact;
> -bool flow_has_vlan;   /* VLAN present at action validation time. */
> -const struct mf_field *field;
> +OFPACT_PADDED_MEMBERS(
> +struct ofpact ofpact;
> +bool flow_has_vlan;   /* VLAN present at action validation time. */
> +const struct mf_field *field;
> +);
> union mf_value value[];  /* Significant value bytes followed by
>   * significant mask bytes. */
> };
> 

Thanks for checking the 32-bit build! Incremental applied.

> I'd suggest ALIGNED_CAST in ofpact_set_field_mask() to make the
> cast-to-void-then-to-real-type trick more greppable.
> 

Done.

> There's a number of instances of code similar to:
> 
> +struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts, mf);
> +memcpy(sf->value, _value, mf->n_bytes);
> +memcpy(ofpact_set_field_mask(sf), _mask, mf->n_bytes);
> 
> so that it might be worthwhile to make a function that combines the
> three steps (or just the memcpy()s?).
> 

I added void * value and mask parameters to ofpact_put_set_field() to this 
effect.

> Acked-by: Ben Pfaff 

Thanks again for the review!

Pushed to master. How about branch-2.6?

  Jarno

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

[ovs-dev] [PATCH v2 4/4] ovn-controller: Convert encaps module back to full processing

[Ryan Moats]

This patch converts the encaps module back to full processing, but
does not remove all persistence of associated data strcutures.

Signed-off-by: Ryan Moats 
---
 ovn/controller/encaps.c | 33 ++---
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index d99ba05..384e8e7 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -412,25 +412,20 @@ encaps_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
 /* Maintain a mapping backwards from encap entries to their parent
  * chassis. Most changes happen at the encap row entry but tunnels need
  * to be established on the basis of the overall chassis. */
-SBREC_CHASSIS_FOR_EACH_TRACKED (chassis_rec, ctx->ovnsb_idl) {
-/* Defer deletion of mapping until we have cleaned up associated
- * ports. */
-if (!sbrec_chassis_is_deleted(chassis_rec)) {
-for (int i = 0; i < chassis_rec->n_encaps; i++) {
-encap_rec = chassis_rec->encaps[i];
-
-struct encap_hash_node *encap_hash_node;
-encap_hash_node = lookup_encap_uuid(_rec->header_.uuid);
-if (encap_hash_node) {
-/* A change might have invalidated our mapping. Process the
- * new version and then iterate over everything to see if 
it
- * is OK. */
-delete_encap_uuid(encap_hash_node);
-poll_immediate_wake();
-}
-
-insert_encap_uuid(_rec->header_.uuid, chassis_rec);
+SBREC_CHASSIS_FOR_EACH (chassis_rec, ctx->ovnsb_idl) {
+for (int i = 0; i < chassis_rec->n_encaps; i++) {
+encap_rec = chassis_rec->encaps[i];
+
+struct encap_hash_node *encap_hash_node;
+encap_hash_node = lookup_encap_uuid(_rec->header_.uuid);
+if (encap_hash_node) {
+/* A change might have invalidated our mapping. Process the
+ * new version and then iterate over everything to see if it
+ * is OK. */
+delete_encap_uuid(encap_hash_node);
 }
+
+insert_encap_uuid(_rec->header_.uuid, chassis_rec);
 }
 }
 
@@ -440,7 +435,7 @@ encaps_run(struct controller_ctx *ctx, const struct 
ovsrec_bridge *br_int,
  * might actually result in the creation of a different type tunnel if
  * that type is preferred. That's OK - when we process the other encap
  * rows, we'll just skip over the new tunnels. */
-SBREC_ENCAP_FOR_EACH_TRACKED (encap_rec, ctx->ovnsb_idl) {
+SBREC_ENCAP_FOR_EACH (encap_rec, ctx->ovnsb_idl) {
 struct encap_hash_node *encap_hash_node;
 struct chassis_hash_node *chassis_hash_node;
 const struct ovsrec_port *port_rec = NULL;
-- 
2.7.4

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

[ovs-dev] [PATCH v2 3/4] Unpersist data structures for address sets.

[Ryan Moats]

With the removal of incremental processing, it is no longer
necessary to persist the data structures for storing address
sets.  Simplify things by removing this complexity.

Side effect: fixed a memory leak in expr_macros_destroy that
is evidenced by this change.

Signed-off-by: Ryan Moats 
---
 ovn/controller/lflow.c | 166 -
 ovn/lib/expr.c |   1 +
 2 files changed, 42 insertions(+), 125 deletions(-)

diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index dc69047..5713c46 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -38,14 +38,10 @@ VLOG_DEFINE_THIS_MODULE(lflow);
 /* Contains "struct expr_symbol"s for fields supported by OVN lflows. */
 static struct shash symtab;
 
-/* Contains an internal expr datastructure that represents an address set. */
-static struct shash expr_address_sets;
-
 void
 lflow_init(void)
 {
 ovn_init_symtab();
-shash_init(_address_sets);
 }
 
 /* Details of an address set currently in address_sets. We keep a cached
@@ -56,54 +52,6 @@ struct address_set {
 size_t n_addresses;
 };
 
-/* struct address_set instances for address sets currently in the symtab,
- * hashed on the address set name. */
-static struct shash local_address_sets = 
SHASH_INITIALIZER(_address_sets);
-
-static int
-addr_cmp(const void *p1, const void *p2)
-{
-const char *s1 = p1;
-const char *s2 = p2;
-return strcmp(s1, s2);
-}
-
-/* Return true if the address sets match, false otherwise. */
-static bool
-address_sets_match(const struct address_set *addr_set,
-   const struct sbrec_address_set *addr_set_rec)
-{
-char **addrs1;
-char **addrs2;
-
-if (addr_set->n_addresses != addr_set_rec->n_addresses) {
-return false;
-}
-size_t n_addresses = addr_set->n_addresses;
-
-addrs1 = xmemdup(addr_set->addresses,
- n_addresses * sizeof addr_set->addresses[0]);
-addrs2 = xmemdup(addr_set_rec->addresses,
- n_addresses * sizeof addr_set_rec->addresses[0]);
-
-qsort(addrs1, n_addresses, sizeof *addrs1, addr_cmp);
-qsort(addrs2, n_addresses, sizeof *addrs2, addr_cmp);
-
-bool res = true;
-size_t i;
-for (i = 0; i <  n_addresses; i++) {
-if (strcmp(addrs1[i], addrs2[i])) {
-res = false;
-break;
-}
-}
-
-free(addrs1);
-free(addrs2);
-
-return res;
-}
-
 static void
 address_set_destroy(struct address_set *addr_set)
 {
@@ -118,79 +66,34 @@ address_set_destroy(struct address_set *addr_set)
 }
 
 static void
-update_address_sets(struct controller_ctx *ctx)
-{
-/* Remember the names of all address sets currently in expr_address_sets
- * so we can detect address sets that have been deleted. */
-struct sset cur_addr_set_names = SSET_INITIALIZER(_addr_set_names);
-
-struct shash_node *node;
-SHASH_FOR_EACH (node, _address_sets) {
-sset_add(_addr_set_names, node->name);
-}
+update_address_sets(struct controller_ctx *ctx,
+struct shash *local_address_sets_p,
+struct shash *expr_address_sets_p)
 
+{
 /* Iterate address sets in the southbound database.  Create and update the
  * corresponding symtab entries as necessary. */
 const struct sbrec_address_set *addr_set_rec;
 SBREC_ADDRESS_SET_FOR_EACH (addr_set_rec, ctx->ovnsb_idl) {
-struct address_set *addr_set =
-shash_find_data(_address_sets, addr_set_rec->name);
-
-bool create_set = false;
-if (addr_set) {
-/* This address set has already been added.  We must determine
- * if the symtab entry needs to be updated due to a change. */
-sset_find_and_delete(_addr_set_names, addr_set_rec->name);
-if (!address_sets_match(addr_set, addr_set_rec)) {
-shash_find_and_delete(_address_sets, addr_set_rec->name);
-expr_macros_remove(_address_sets, addr_set_rec->name);
-address_set_destroy(addr_set);
-addr_set = NULL;
-create_set = true;
+/* Create a symbol that resolves to the full set of addresses.
+ * Store it in address_sets to remember that we created this
+ * symbol. */
+struct address_set *addr_set = xzalloc(sizeof *addr_set);
+addr_set->n_addresses = addr_set_rec->n_addresses;
+if (addr_set_rec->n_addresses) {
+addr_set->addresses = xmalloc(addr_set_rec->n_addresses
+  * sizeof addr_set->addresses[0]);
+size_t i;
+for (i = 0; i < addr_set_rec->n_addresses; i++) {
+addr_set->addresses[i] = xstrdup(addr_set_rec->addresses[i]);
 }
-} else {
-/* This address set is not yet in the symtab, so add it. */
-create_set = true;
 }
+shash_add(local_address_sets_p, 

[ovs-dev] [PATCH v2 2/4] ovn-controller: add quiet mode

[Ryan Moats]

As discussed in [1], what the incremental processing code
actually accomplished was that the ovn-controller would
be "quiet" and not burn CPU when things weren't changing.
This patch set recreates this state by calculating whether
changes have occured that would require a full calculation
to be performed.  It does this by persisting a copy of
the localvif_to_ofport and tunnel information in the
controller module, rather than in the physical.c module
as was the case with previous commits.

[1] http://openvswitch.org/pipermail/dev/2016-August/078272.html

Signed-off-by: Ryan Moats 
---
 ovn/controller/ofctrl.c |   2 +
 ovn/controller/ovn-controller.c |  73 -
 ovn/controller/ovn-controller.h |   1 +
 ovn/controller/patch.c  |  11 +
 ovn/controller/physical.c   | 100 
 ovn/controller/physical.h   |   5 ++
 6 files changed, 131 insertions(+), 61 deletions(-)

diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index d8e111d..5b1434e 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -366,6 +366,8 @@ run_S_CLEAR_FLOWS(void)
 queue_msg(encode_group_mod());
 ofputil_uninit_group_mod();
 
+force_full_process();
+
 /* Clear existing groups, to match the state of the switch. */
 if (groups) {
 ovn_group_table_clear(groups, true);
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index c2e667b..cb0ae40 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -304,6 +304,23 @@ get_nb_cfg(struct ovsdb_idl *idl)
 return sb ? sb->nb_cfg : 0;
 }
 
+/* Contains mapping of localvifs to OF ports for detecting if the physical
+ * configuration of the switch has changed. */
+static struct simap localvif_to_ofport =
+SIMAP_INITIALIZER(_to_ofport);
+
+/* Contains the list of known tunnels. */
+static struct hmap tunnels = HMAP_INITIALIZER();
+
+/* The last sequence number seen from the southbound IDL. */
+static unsigned int seqno = 0;
+
+void
+force_full_process(void)
+{
+seqno = 0;
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -427,39 +444,51 @@ main(int argc, char *argv[])
 _lports);
 }
 
+enum mf_field_id mff_ovn_geneve;
+bool physical_change = true;
 if (br_int && chassis_id) {
+mff_ovn_geneve = ofctrl_run(br_int);
+physical_change = detect_and_save_physical_changes(
+_to_ofport, , mff_ovn_geneve, br_int,
+chassis_id);
+unsigned int cur_seqno = ovsdb_idl_get_seqno(ovnsb_idl_loop.idl);
+
 patch_run(, br_int, chassis_id, _datapaths,
   _datapaths);
 
 static struct lport_index lports;
-static struct mcgroup_index mcgroups;
 lport_index_init(, ctx.ovnsb_idl);
-mcgroup_index_init(, ctx.ovnsb_idl);
-
-enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
-
-pinctrl_run(, , br_int, chassis_id, _datapaths);
+pinctrl_run(, , br_int, chassis_id,
+_datapaths);
 update_ct_zones(_lports, _datapaths, _zones,
 ct_zone_bitmap);
 
-struct hmap flow_table = HMAP_INITIALIZER(_table);
-lflow_run(, , , _datapaths,
-  _datapaths, _table, _zones,
-  _table);
-
-physical_run(, mff_ovn_geneve,
- br_int, chassis_id, _zones, _table,
- _datapaths, _datapaths);
-
-ofctrl_put(_table, get_nb_cfg(ctx.ovnsb_idl));
-hmap_destroy(_table);
-if (ctx.ovnsb_idl_txn) {
-int64_t cur_cfg = ofctrl_get_cur_cfg();
-if (cur_cfg && cur_cfg != chassis->nb_cfg) {
-sbrec_chassis_set_nb_cfg(chassis, cur_cfg);
+
+if (physical_change || seqno < cur_seqno) {
+seqno = cur_seqno;
+
+static struct mcgroup_index mcgroups;
+mcgroup_index_init(, ctx.ovnsb_idl);
+
+struct hmap flow_table = HMAP_INITIALIZER(_table);
+lflow_run(, , , _datapaths,
+  _datapaths, _table, _zones,
+  _table);
+
+physical_run(, mff_ovn_geneve,
+ br_int, chassis_id, _zones, _table,
+ _datapaths, _datapaths);
+
+ofctrl_put(_table, get_nb_cfg(ctx.ovnsb_idl));
+hmap_destroy(_table);
+if (ctx.ovnsb_idl_txn) {
+int64_t cur_cfg = ofctrl_get_cur_cfg();
+if (cur_cfg && cur_cfg != chassis->nb_cfg) {
+sbrec_chassis_set_nb_cfg(chassis, cur_cfg);
+}
 }
+mcgroup_index_destroy();
 }
-

[ovs-dev] [PATCH v2 1/4] ovn-controller: Back out incremental processing

[Ryan Moats]

As [1] indicates, incremental processing hasn't resulted
in an improvement worth the complexity it has added.
This patch backs out all of the code specific to incremental
processing, along with the persisting of OF flows,
logical ports, multicast groups, all_lports, local and patched
datapaths.

Persisted objects in the ovn/controller/physical.c module will
be used by a future patch set to determine if physical changes
have occurred.

Future patch sets in the series will convert
the ovn/controller/encaps.c module back to full processing
and remove the persistance of address sets in the
ovn/controller/lflow.c module.

[1] http://openvswitch.org/pipermail/dev/2016-August/078272.html

Signed-off-by: Ryan Moats 
---
 include/ovn/actions.h   |   4 -
 ovn/controller/binding.c| 154 ++
 ovn/controller/binding.h|   1 -
 ovn/controller/encaps.c | 111 ++---
 ovn/controller/lflow.c  | 102 +++-
 ovn/controller/lflow.h  |   4 +-
 ovn/controller/lport.c  | 220 -
 ovn/controller/lport.h  |  24 +--
 ovn/controller/ofctrl.c | 353 +++-
 ovn/controller/ofctrl.h |  16 +-
 ovn/controller/ovn-controller.c |  61 ---
 ovn/controller/patch.c  |   6 -
 ovn/controller/physical.c   | 175 +---
 ovn/controller/physical.h   |   3 +-
 ovn/lib/actions.c   |   1 -
 tests/ovn.at|  55 +++
 tests/system-ovn.at |  45 ++---
 17 files changed, 387 insertions(+), 948 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index e2a716a..b678d33 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -298,7 +298,6 @@ struct group_table {
 struct group_info {
 struct hmap_node hmap_node;
 struct ds group;
-struct uuid lflow_uuid;
 uint32_t group_id;
 };
 
@@ -412,9 +411,6 @@ struct ovnact_encode_params {
 /* A struct to figure out the group_id for group actions. */
 struct group_table *group_table;
 
-/* The logical flow uuid that drove this action. */
-struct uuid lflow_uuid;
-
 /* OVN maps each logical flow table (ltable), one-to-one, onto a physical
  * OpenFlow flow table (ptable).  A number of parameters describe this
  * mapping and data related to flow tables:
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index c26007d..0353a7b 100644
--- a/ovn/controller/binding.c
+++ b/ovn/controller/binding.c
@@ -30,18 +30,6 @@
 
 VLOG_DEFINE_THIS_MODULE(binding);
 
-/* A set of the iface-id values of local interfaces on this chassis. */
-static struct sset local_ids = SSET_INITIALIZER(_ids);
-
-/* When this gets set to true, the next run will re-check all binding records. 
*/
-static bool process_full_binding = false;
-
-void
-binding_reset_processing(void)
-{
-process_full_binding = true;
-}
-
 void
 binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
 {
@@ -64,16 +52,12 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
  _interface_col_ingress_policing_burst);
 }
 
-static bool
+static void
 get_local_iface_ids(const struct ovsrec_bridge *br_int,
 struct shash *lport_to_iface,
 struct sset *all_lports)
 {
 int i;
-bool changed = false;
-
-struct sset old_local_ids = SSET_INITIALIZER(_local_ids);
-sset_clone(_local_ids, _ids);
 
 for (i = 0; i < br_int->n_ports; i++) {
 const struct ovsrec_port *port_rec = br_int->ports[i];
@@ -93,53 +77,9 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int,
 continue;
 }
 shash_add(lport_to_iface, iface_id, iface_rec);
-if (!sset_find_and_delete(_local_ids, iface_id)) {
-sset_add(_ids, iface_id);
-sset_add(all_lports, iface_id);
-changed = true;
-}
+sset_add(all_lports, iface_id);
 }
 }
-
-/* Any item left in old_local_ids is an ID for an interface
- * that has been removed. */
-if (!changed && !sset_is_empty(_local_ids)) {
-changed = true;
-
-const char *cur_id;
-SSET_FOR_EACH(cur_id, _local_ids) {
-sset_find_and_delete(_ids, cur_id);
-sset_find_and_delete(all_lports, cur_id);
-}
-}
-
-sset_destroy(_local_ids);
-
-return changed;
-}
-
-static struct local_datapath *
-local_datapath_lookup_by_uuid(struct hmap *hmap_p, const struct uuid *uuid)
-{
-struct local_datapath *ld;
-HMAP_FOR_EACH_WITH_HASH(ld, uuid_hmap_node, uuid_hash(uuid), hmap_p) {
-if (uuid_equals(>uuid, uuid)) {
-return ld;
-}
-}
-return NULL;
-}
-
-static void
-remove_local_datapath(struct hmap *local_datapaths, struct local_datapath *ld)
-{
-if (ld->logical_port) {
-free(ld->logical_port);
-ld->logical_port = NULL;

[ovs-dev] [PATCH v2 0/4] ovn-controller: Replace incremental processing with quiet mode

[Ryan Moats]

This patch set removes incremental processing and replaces it
with quiet mode: ovn-controller first tests to see if recalculation
is necessary and if so, does a full calculation.

Side effect of this is that almost all data persistence is also reverted.
The exception is the persistence in the encaps.c module - it's not clear
how to revert that while still maintaining 100% unit test pass.

v1->v2:
  removed poll_immediate_wake() from encaps.c that will lead to
  ovn-controller pegging a cpu.

Ryan Moats (4):
  ovn-controller: Back out incremental processing
  ovn-controller: add quiet mode
  Unpersist data structures for address sets.
  ovn-controller: Convert encaps module back to full processing

 include/ovn/actions.h   |   4 -
 ovn/controller/binding.c| 154 ++
 ovn/controller/binding.h|   1 -
 ovn/controller/encaps.c | 144 +++-
 ovn/controller/lflow.c  | 258 +++--
 ovn/controller/lflow.h  |   4 +-
 ovn/controller/lport.c  | 220 -
 ovn/controller/lport.h  |  24 +--
 ovn/controller/ofctrl.c | 353 +++-
 ovn/controller/ofctrl.h |  16 +-
 ovn/controller/ovn-controller.c | 106 
 ovn/controller/ovn-controller.h |   1 +
 ovn/controller/patch.c  |  17 +-
 ovn/controller/physical.c   | 263 +++---
 ovn/controller/physical.h   |   8 +-
 ovn/lib/actions.c   |   1 -
 ovn/lib/expr.c  |   1 +
 tests/ovn.at|  55 +++
 tests/system-ovn.at |  45 ++---
 19 files changed, 548 insertions(+), 1127 deletions(-)

-- 
2.7.4

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

Re: [ovs-dev] [PATCH v2] ovn-nbctl: Add LB commands.

[Guru Shetty]

On 29 August 2016 at 20:12, nickcooper-zhangtonghao <
nickcooper-zhangtong...@opencloud.tech> wrote:

> Add a name column for the load balancer.  This must be unique among
> all load balancers.  This name has no special meaning or purpose
> other than to provide convenience for human interaction with the
> ovn-nb database.  This patch provides the command line to
> create load-balancer and add the unit tests.
>
I think you missed one of my points in the previous review. I want to be
able to create the load balancer independently.
 i..e I don't want to wait till I add it to a switch. This helps in
creating a load-balancer and adding it to multiple switches. Also, I have
code sent out for review to add the load-balancer to the gateway router
too. So the commands should work with that too.

So with your patch, can I create a load balancer independently and add it
to multiple switches?


>
> Signed-off-by: nickcooper-zhangtonghao  loud.tech>
> ---
>  ovn/ovn-nb.ovsschema  |   5 +-
>  ovn/ovn-nb.xml|   6 ++
>  ovn/utilities/ovn-nbctl.8.xml |  35 +
>  ovn/utilities/ovn-nbctl.c | 163 ++
> +++-
>  tests/ovn-nbctl.at|  44 
>  tests/system-ovn.at   |  13 ++--
>  6 files changed, 255 insertions(+), 11 deletions(-)
>
> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
> index 456ae98..87ce15f 100644
> --- a/ovn/ovn-nb.ovsschema
> +++ b/ovn/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>  "name": "OVN_Northbound",
>  "version": "5.3.1",
> -"cksum": "1921908091 9353",
> +"cksum": "360001470 9398",
>  "tables": {
>  "NB_Global": {
>  "columns": {
> @@ -92,6 +92,7 @@
>  "isRoot": true},
>  "Load_Balancer": {
>  "columns": {
> +"name": {"type": "string"},
>  "vips": {
>  "type": {"key": "string", "value": "string",
>   "min": 0, "max": "unlimited"}},
> @@ -102,7 +103,7 @@
>  "external_ids": {
>  "type": {"key": "string", "value": "string",
>   "min": 0, "max": "unlimited"}}},
> -"isRoot": true},
> +"isRoot": false},
>  "ACL": {
>  "columns": {
>  "priority": {"type": {"key": {"type": "integer",
> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
> index 5719e74..c3666f5 100644
> --- a/ovn/ovn-nb.xml
> +++ b/ovn/ovn-nb.xml
> @@ -660,6 +660,12 @@
>Each row represents one load balancer.
>  
>
> +
> +  A name for the load balancer.  This must be unique among all load
> balancers.
> +  This name has no special meaning or purpose other than to provide
> convenience
> +  for human interaction with the ovn-nb database.
> +
> +
>  
>
>  A map of virtual IPv4 addresses (and an optional port number with
> diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
> index d44f039..fa908b7 100644
> --- a/ovn/utilities/ovn-nbctl.8.xml
> +++ b/ovn/utilities/ovn-nbctl.8.xml
> @@ -102,6 +102,41 @@
>
>  
>
> +Logical Switch LB Commands
> +
> +  [--may-exist] lb-add
> switch lb vip ips
> [protocol]
> +  
> +Adds the specified LB to switch.  We should assign
> lb
> +a virtual IPv4 address (and an optional port number with : as a
> separator)
> +and the corresponding endpoint IPv4 addresses (and optional port
> numbers
> +with : as separators) separated by commas.  The optional argument
> protocol
> +must be either tcp or udp.  This
> argument is useful
> +when a port number is provided as part of the vip.  If
> the protocol
> +is unspecified and a port number is provided as part of
> vip,
> +OVN assumes the protocol to be tcp.  It
> is an error if a load-balancer
> +named lb already exists, unless
> --may-exist is specified.
> +The following example adds a load-balancer with
> protocol udp:
> +
> +  lb-add ls0 lb0 30.0.0.10:80 192.168.10.10:80,
> 192.168.10.20:80,192.168.10.30:80 udp
> +
> +  
> +
> +  [--if-exists] lb-del
> switch [lb]
> +  
> +Deletes LBs from switch.  If only
> +switch is supplied, all the LBs from the logical
> +switch are deleted.  If lb is also specified,
> +then the lb will be deleted only from the logical
> switch.
> +It is an error if lb does not exist, unless
> --if-exists
> +is specified.
> +  
> +
> +  lb-list switch
> +  
> +Lists the LBs on switch.
> +  
> +
> +
>  Logical Switch Port Commands
>  
>[--may-exist] lsp-add
> switch port
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index d6d64ea..fc02918 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -331,6 

[ovs-dev] my photo

[Margret]

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

[ovs-dev] my photo

[Amber]

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

[ovs-dev] Fax

[Antoinette]

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

[ovs-dev] photos

[Rosemarie]

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

[ovs-dev] Returned mail: see transcript for details

[jaida . l]

The original message was received at Wed, 31 Aug 2016 18:32:51 +0530 from 
6.228.10.129

- The following addresses had permanent fatal errors -


- Transcript of session follows -
... while talking to 149.119.230.11:
554 Service unavailable; [172.144.23.229] blocked using relays.osirusoft.com
Session aborted, reason: lost connection

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

[ovs-dev] MESSAGE COULD NOT BE DELIVERED

[Mail Delivery Subsystem]

The message was not delivered due to the following reason:

Your message was not delivered because the destination server was
unreachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message could not be delivered within 5 days:
Host 212.9.34.102 is not responding.

The following recipients could not receive this message:


Please reply to postmas...@openvswitch.org
if you feel this message to be in error.

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

Re: [ovs-dev] [PATCH] ovn: ovn-ctl support to start ovn db servers in standby mode

[Babu Shanmugam]

On Wednesday 31 August 2016 02:42 AM, Andy Zhou wrote:



On Thu, Aug 25, 2016 at 6:48 AM, > wrote:


This patch adds support to start_ovsdb() function in ovn-ctl to
start the
ovn db servers in standby mode. This can be done in the following ways
1. Use parameters --ovn-nb-sync-from-addr and
--ovn-sb-sync-from-addr to
   set the addresses of the master server.
2. Create files $etcdir/ovnnb-master.conf and
$etcdir/ovnsb-master.conf
   with the tcp url of the master servers.

If --ovn-nb-sync-from-addr and --ovn-sb-sync-from-addr is used, it
will
overwrite the contents in the $etcdir/*.conf and use that server
as the
master.

Additional functions to promote a standby server to master and demote
master server to standby mode are also added in this patch

Signed-off-by: Babu Shanmugam >


In the context of OVSDB HA, we have been calling the ovsdb-server pair 
as active/backup.
It would be nice if we can avoid using another term "master".  I have 
no strong preference

for "active", just want to avoid using another term.


I will change it in the next revision.

It seems that if demote_ovssb() is followed by restart_ovsdb(),  the 
role switching

won't be respected. true?

You are right Andy. I will fix this too.

Thank you

Thanks.


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

[ovs-dev] IMG

[Rosemary]

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

Re: [ovs-dev] [replication SMv2 7/7] ovsdb: Replication usability improvements

[Numan Siddique]

On Wed, Aug 31, 2016 at 12:03 AM, Andy Zhou  wrote:

>
>
> On Tue, Aug 30, 2016 at 4:17 AM, Numan Siddique 
> wrote:
>
>>
>>
>> On Tue, Aug 30, 2016 at 1:11 AM, Andy Zhou  wrote:
>>
>>>
>>>
>>> On Mon, Aug 29, 2016 at 3:14 AM, Numan Siddique 
>>> wrote:
>>>


 On Sat, Aug 27, 2016 at 4:45 AM, Andy Zhou  wrote:

> Added the '--no-sync' option base on feedbacks of current
> implementation.
>
> Added appctl command "ovsdb-server/sync-status" based on feedbacks
> of current implementation.
>
> Added a test to simulate the integration of HA manager with OVSDB
> server using replication.
>
> Other documentation and API improvements.
>
> Signed-off-by: Andy Zhou 
> --
>
> I hope to get some review comments on the command line and appctl
> interfaces for replication. Since 2.6 is the first release of those
> interfaces, it is easier to making changes, compare to future
> releases.
>
> 
> v1->v2: Fix creashes reported at:
> http://openvswitch.org/pipermail/dev/2016-August/078591.html
> ---
>

 ​I haven't tested these patches yet. This patch seems to have a white
 space warning when applied.

>>> Thanks for the reported. I will fold the fix in the next version when
>>> posting.
>>>
>>> In case it helps, you can also access the patches from my private repo
>>> at:
>>>   https://github.com/azhou-nicira/ovs-review/tree/ovsdb-replic
>>> ation-sm-v2
>>>
>>>
>> ​
>> Hi Andy,
>> ​
>> I am seeing the below crash when
>>
>> ​  - The ovsdb-server changes from
>> ​master to standby and the active-ovsdb-server it is about to connect to
>> is killed just before that or it is not reachable.
>>
>> ​  -
>> ​The pacemaker OCF script calls the sync-status cmd soon after that.
>>
>>
>> ​Please let me know if you need more information.​
>>
>>
>> ​Core was generated by `ovsdb-server -vdbg 
>> --log-file=/opt/stack/logs/ovsdb-server-sb.log
>> --remote=puni'.
>> Program terminated with signal SIGSEGV, Segmentation fault.
>> #0  0x0041241d in replication_status () at ovsdb/replication.c:875
>> 875SHASH_FOR_EACH (node, replication_dbs) {
>> Missing separate debuginfos, use: dnf debuginfo-install
>> glibc-2.23.1-10.fc24.x86_64 openssl-libs-1.0.2h-3.fc24.x86_64
>> (gdb) bt
>> #0  0x0041241d in replication_status () at ovsdb/replication.c:875
>> #1  0x00406eda in ovsdb_server_get_sync_status (conn=0x1421fd0,
>> argc=, argv=, config_=)
>> at ovsdb/ovsdb-server.c:1480
>> #2  0x004324ee in process_command (request=0x1421f30,
>> conn=0x1421fd0) at lib/unixctl.c:313
>> #3  run_connection (conn=0x1421fd0) at lib/unixctl.c:347
>> #4  unixctl_server_run (server=server@entry=0x141e140) at
>> lib/unixctl.c:400
>> #5  0x00405bdc in main_loop (is_backup=0x7fff08062256,
>> exiting=0x7fff08062257, run_process=0x0, remotes=0x7fff080622a0,
>> unixctl=0x141e140,
>> all_dbs=0x7fff080622e0, jsonrpc=0x13f6f00) at ovsdb/ovsdb-server.c:182
>> #6  main (argc=, argv=) at
>> ovsdb/ovsdb-server.c:430​
>>
>> Numan, thanks for the report. I think I spotted the bug:
>
> Currently, when replication state machine is reset,  the state update
> takes place after a round of main loop run. this time lag
> could lead to the back trace in case the unixctl commands was issued
> during this time lag.  I have a fix that add another
> state to represent the reset condition.  The fix is at:
>
> https://github.com/azhou-nicira/ovs-review/tree/ovsdb-replication-sm-v3
>
> Would you please let me know if this version works any better?. Thanks!
>

​Sure. I would test and let you know.

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

Re: [ovs-dev] [PATCH] ovs-monitor-ipsec: Fix flake8-check semicolon error

[Richard Theis]

Amitabha Biswas  wrote on 08/31/2016 01:35:07 AM:

> From: Amitabha Biswas 
> To: dev@openvswitch.org
> Cc: russ...@ovn.org, Richard Theis/Rochester/IBM@IBMUS, Amitabha 
> Biswas/San Jose/IBM@IBMUS
> Date: 08/31/2016 01:35 AM
> Subject: [PATCH] ovs-monitor-ipsec: Fix flake8-check semicolon error
> 
> ---
>  debian/ovs-monitor-ipsec | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
> index e6617b0..6bc26aa 100755
> --- a/debian/ovs-monitor-ipsec
> +++ b/debian/ovs-monitor-ipsec
> @@ -333,7 +333,7 @@ class IPsec(object):
>  self.call_ip_xfrm(["policy", "add", "src", "0.0.0.0/0", "dst",
> "0.0.0.0/0", "proto", "gre", "dir", "out",
> "mark", IPSEC_MARK, "mask", IPSEC_MARK,
> -   "action", "block", "priority", 
"4294967295"]);
> +   "action", "block", "priority", 
"4294967295"])
> 
>  def spd_add(self, local_ip, remote_ip):
>  cmds = ("spdadd %s %s gre -P out ipsec 
esp/transport//require;\n" %
> -- 
> 2.7.4 (Apple Git-66)
> 

The OpenStack networking-ovn gate should be fixed with this change.
This change was tested as part of OpenStack patch set
https://review.openstack.org/#/c/363305/.

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

Re: [ovs-dev] [PATCH] ovs-monitor-ipsec: Fix flake8-check semicolon error

[Numan Siddique]

On Wed, Aug 31, 2016 at 12:05 PM, Amitabha Biswas 
wrote:

> ---
>  debian/ovs-monitor-ipsec | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
> index e6617b0..6bc26aa 100755
> --- a/debian/ovs-monitor-ipsec
> +++ b/debian/ovs-monitor-ipsec
> @@ -333,7 +333,7 @@ class IPsec(object):
>  self.call_ip_xfrm(["policy", "add", "src", "0.0.0.0/0", "dst",
> "0.0.0.0/0", "proto", "gre", "dir", "out",
> "mark", IPSEC_MARK, "mask", IPSEC_MARK,
> -   "action", "block", "priority", "4294967295"]);
> +   "action", "block", "priority", "4294967295"])
>
>
​Thanks for the patch. This fixes the compilation error which I am seeing.

May be you can update the below in the commit message

Fixes:
19cd0a87


Acked-by: Numan Siddique 

​


>  def spd_add(self, local_ip, remote_ip):
>  cmds = ("spdadd %s %s gre -P out ipsec esp/transport//require;\n"
> %
> --
> 2.7.4 (Apple Git-66)
>
> ___
> 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] photos

[Emma]

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

[ovs-dev] Image

[Juliana]

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

[ovs-dev] Fax

[Briana]

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

[ovs-dev] IMG

[Karin]

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

[ovs-dev] [PATCH] datapath-windows: remove invalid ASSERT in Flow.c

[Nithin Raju]

Since the Geneve changes, the key->l2.offset will no longer be 0 when
the tunnel key is valid within the OVS flow key. key->l2.offset would
be determined by the amount of tunnel options.

Signed-off-by: Nithin Raju 
---
 datapath-windows/ovsext/DpInternal.h | 9 ++---
 datapath-windows/ovsext/Flow.c   | 1 -
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/datapath-windows/ovsext/DpInternal.h 
b/datapath-windows/ovsext/DpInternal.h
index 22599a0..f62fc55 100644
--- a/datapath-windows/ovsext/DpInternal.h
+++ b/datapath-windows/ovsext/DpInternal.h
@@ -157,17 +157,20 @@ typedef union OvsIPv4TunnelKey {
 uint64_t attr[NUM_PKT_ATTR_REQUIRED];
 } OvsIPv4TunnelKey; /* Size of 280 byte. */
 
-__inline uint8_t TunnelKeyGetOptionsOffset(const OvsIPv4TunnelKey *key)
+static __inline uint8_t
+TunnelKeyGetOptionsOffset(const OvsIPv4TunnelKey *key)
 {
 return TUN_OPT_MAX_LEN - key->tunOptLen;
 }
 
-__inline uint8_t* TunnelKeyGetOptions(OvsIPv4TunnelKey *key)
+static __inline uint8_t *
+TunnelKeyGetOptions(OvsIPv4TunnelKey *key)
 {
 return key->tunOpts + TunnelKeyGetOptionsOffset(key);
 }
 
-__inline uint16_t TunnelKeyGetRealSize(OvsIPv4TunnelKey *key)
+static __inline uint16_t
+TunnelKeyGetRealSize(OvsIPv4TunnelKey *key)
 {
 return sizeof(OvsIPv4TunnelKey) - TunnelKeyGetOptionsOffset(key);
 }
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 7a57f96..439fb28 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -2595,7 +2595,6 @@ OvsHashFlow(const OvsFlowKey *key)
 UINT8 *start;
 
 ASSERT(key->tunKey.dst || offset == sizeof(OvsIPv4TunnelKey));
-ASSERT(!key->tunKey.dst || offset == 0);
 start = (UINT8 *)key + offset;
 return OvsJhashBytes(start, size, 0);
 }
-- 
2.6.2

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

[ovs-dev] one issue about vhost xstats with/without CRC

[Yang, Zhiyong]

Hi, all:

Physical NIC has a set of hardware counters, such as
u64 prc64;
u64 prc127;
u64 prc255; etc.
DPDK counts the prc64 in two ways. Physical NIC counts prc64 with CRC by
hardware. Virtio computes the counter like prc64 without CRC. This will cause
the conflict, when a 64 packet from outer network is sent to VM(virtio), NIC
will show prc64 + 1, virtio will actually receive the 64-4(CRC) = 60 bytes pkt,
undersize(<64) counter will be increased, since the Length of the packet will
be subtracted CRC by hardware(NIC offload enable) or software((NIC offload
disable)).

if vhost xstats implements like NIC, It will solve the  consistency issue. But 
when
running vm to vm or virtio/vhost loopback test, xstats counters will count
non-existent CRC bytes .

How should Packets length of virtio/vhost be counted from OVS perspective?
With or without CRC?

Thanks
Zhiyong

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

[ovs-dev] How to make ovs bridge come up on boot

[Hussain Hakimuddin Nagri]

Hi everyone,

I am trying to figure out how to bring my br0 bridge up every time my
server boots.
usually i have to follow these steps,

sudo ovs-vsctl add-br br0
> sudo ovs-vsctl add-port br0 enp1s0
> sudo ifconfig enp1s0 0.0.0.0
> sudo dhclient br0
>

I tried to follow
https://github.com/openvswitch/ovs/blob/master/debian/openvswitch-switch.README.Debian
but it does not seem to work.

Do I have to make entries in /etc/network/interface or create some kind of
script?

I am on Ubuntu 16.04 and ovs-vsctl (Open vSwitch) 2.5.0 if that is of any
relevance.

Thank you.

-Regards-
Hussain Nagri

H



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

[ovs-dev] Returned mail: Data format error

[xbustelo]

The original message was received at Wed, 31 Aug 2016 13:42:20 +0530
from usal.es [29.197.93.198]

- The following addresses had permanent fatal errors -


- Transcript of the session follows -
... while talking to mail server openvswitch.org.:
554 Service unavailable; [91.70.153.137] blocked using bl.spamcop.net, reason: 
Blocked
Session aborted

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

[ovs-dev] Status

[bobkat1]

This message was undeliverable due to the following reason(s):

Your message was not delivered because the destination server was
not reachable within the allowed queue period. The amount of time
a message is queued before it is returned depends on local configura-
tion parameters.

Most likely there is a network problem that prevented delivery, but
it is also possible that the computer is turned off, or does not
have a mail system running right now.

Your message could not be delivered within 3 days:
Host 135.16.101.246 is not responding.

The following recipients could not receive this message:


Please reply to postmas...@openvswitch.org
if you feel this message to be in error.

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

Re: [ovs-dev] [PATCH V2 02/10] python tests: Skip python tests that kill the python daemon

[Paul Boca]

Hi Guru,

There are other tests that check if the daemon is running fine.
In my opinion we could skip this test on Windows and let it on Linux to run.

Paul

From: Guru Shetty [mailto:g...@ovn.org]
Sent: Tuesday, August 30, 2016 6:18 PM
To: Paul Boca
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH V2 02/10] python tests: Skip python tests that 
kill the python daemon



On 30 August 2016 at 05:00, Paul Boca 
> wrote:
If the python script is killed with `kill` command, the atexit
handler doesn't gets executed on Windows.
The kill of the process is done using NtTerminateProcess which
doesn't sends a signal to the process itself, if just terminates the process
from kernel mode.

Signed-off-by: Paul-Daniel Boca 
>

Instead of skipping the test, why not just skip the line that tests for pid 
after the process is killed. I imagine that the test is still useful in other 
respects, like whether detach works fine?

---
V2: Initial commit
---
 tests/daemon-py.at | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/daemon-py.at 
b/tests/daemon-py.at
index 96dea07..11833c8 100644
--- a/tests/daemon-py.at
+++ b/tests/daemon-py.at
@@ -126,6 +126,8 @@ DAEMON_MONITOR_RESTART_PYN([Python3], [$HAVE_PYTHON3], 
[$PYTHON3])
 m4_define([DAEMON_DETACH_PYN],
   [AT_SETUP([daemon --detach - $1])
AT_SKIP_IF([test $2 = no])
+   # Skip this test for Windows, the pid file not removed if the daemon is 
killed
+   AT_SKIP_IF([test "$IS_WIN32" = "yes"])
AT_CAPTURE_FILE([pid])
# Start the daemon and make sure that the pidfile exists immediately.
# We don't wait for the pidfile to get created because the daemon is
--
2.7.2.windows.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[ovs-dev] Delivery reports about your e-mail

[Post Office]

The original message was received at Wed, 31 Aug 2016 12:57:31 +0530 from 
openvswitch.org [210.6.150.164]

- The following addresses had permanent fatal errors -
dev@openvswitch.org



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

[ovs-dev] [patch v11 0/2] QOS updates with DSCP support

[bschanmu]

v10 -> v11
 - Moved the DSCP marking at a later stage as suggested by Mickey Spiegel
 - Update the documentation for the DSCP in ovn northd's man page template

Babu Shanmugam (2):
  Check and allocate free qdisc queue id for ports with qos parameters
  DSCP marking on packets egressing VIF interface

 include/ovn/actions.h   |  17 +++-
 ovn/controller/binding.c| 238 +++-
 ovn/lib/actions.c   |  42 
 ovn/lib/logical-fields.c|   2 +-
 ovn/northd/ovn-northd.8.xml |  30 --
 ovn/northd/ovn-northd.c | 207 --
 ovn/ovn-nb.xml  |  14 ++-
 ovn/ovn-sb.xml  |  43 +++-
 tests/ovn.at|  81 +++
 9 files changed, 601 insertions(+), 73 deletions(-)

-- 
1.9.1

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