Re: [ovs-dev] [PATCH]: bfd: display last wall clock time of last flap

2015-06-25 Thread Justin Pettit

> On Jun 25, 2015, at 3:08 AM, Sabyasachi Sengupta 
>  wrote:
> 
> [root@rtr-29-225-196-232 ~]# ovs-appctl bfd/show
>  port3.4094 
>Forwarding: true
>Detect Multiplier: 3
>Concatenated Path Down: false
>TX Interval: Approx 500ms
>RX Interval: Approx 500ms
>Detect Time: now -1116ms
>Next TX Time: now -436ms
>Last TX Time: now +19ms
>Last Flap Time: 2015-06-25 00:36:22.372
> ...
> I understand that this will be based on what ovs-vswitchd 'thinks' as to when 
> the last flap occurred. Its still not the accurate information as 
> ovs-vswitchd might itself have restarted in between the last flap and when 
> the user actually reads it. This probably means that we save this info in 
> ovsdb and read it through a separate CLI, but probably we can keep it simple 
> for now?

I tend to prefer the number of seconds or milliseconds.  However, if we do go 
with wall clock, we should at least put in the offset to GMT.  Otherwise, when 
you get an off-line bug report, it can be difficult to know when the issue 
happened when trying to correlate it to other logs.

--Justin


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


[ovs-dev] [PATCH v4] ovs-ofctl: replace-flows and diff-flows support for multiple tables

2015-06-25 Thread Shashank Shanbhag
From: Shashank Shanbhag 

Fix replace-flows and diff-flows to modify/diff flows in multiple tables.
Add a --tables(-T) option that allows the user to specify a comma-separated
list of table indexes to replace/diff.

Signed-off-by: Shashank Shanbhag 
Acked-by: Romain Lenglet 
---
 AUTHORS  |   1 +
 NEWS |   1 +
 tests/ovs-ofctl.at   | 129 ++
 utilities/ovs-ofctl.8.in |  20 ++-
 utilities/ovs-ofctl.c| 442 ---
 5 files changed, 498 insertions(+), 95 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 2240576..cfdb9b5 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -156,6 +156,7 @@ Scott Lowe  scott.l...@scottlowe.org
 Scott Mann  sdm...@gmail.com
 Selvamuthukumar smku...@merunetworks.com
 Shan Weidavids...@tencent.com
+Shashank Shanbhag   shashank.shanb...@gmail.com
 Shih-Hao Li s...@nicira.com
 Shu Shenshu.s...@radisys.com
 Simon Hormanho...@verge.net.au
diff --git a/NEWS b/NEWS
index 395444b..ba567f5 100644
--- a/NEWS
+++ b/NEWS
@@ -116,6 +116,7 @@ v2.4.0 - xx xxx 
- Support for STT tunneling.
- ovs-sim: New developer tool for simulating multiple OVS instances.
  See ovs-sim(1) for more information.
+   - ovs-ofctl: replace-flows and diff-flows now operate on multiple tables.
 
 
 v2.3.0 - 14 Aug 2014
diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index f20f2da..e457b3e 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2789,27 +2789,82 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sed 
'/ST_FLOW reply/d' | sort
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-
 dnl Importance parameter added in OF1.4.
 dnl This validates whether flows with importance
 dnl parameter are getting replaced with "replace-flows" or
 dnl not by validating dump-flows output after replace with the expected output.
+dnl MOD_FLOW does not modify importance field - ONF EXT-496.
 
 AT_SETUP([ovs-ofctl replace-flows with importance])
 OVS_VSWITCHD_START
 
 dnl Add flows to br0 with importance via OF1.4+. For more details refer 
"ovs-ofctl rule with importance" test case.
-for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i,actions=drop"; 
done > add-flows.txt
+for i in 1 2 3 4 5 6 7 8; do echo "dl_vlan=$i,importance=$i actions=drop"; 
done | sort > add-flows.txt
 AT_CHECK([ovs-ofctl -O OpenFlow14 add-flows br0 add-flows.txt])
 
-dnl Replace some flows in the bridge.
-for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + 10`,actions=drop"; 
done > replace-flows.txt
+dnl Modify odd numbered flows. Leave even numbered ones alone.
+for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then 
importance=`expr $i + 10`; else importance=$i; fi; echo " 
importance=$importance, dl_vlan=$i actions=drop"; done | sort > 
replace-flows.txt
+AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 replace-flows.txt])
+
+dnl Dump the flows and compare them against expected output.
+
+for i in 1 2 3 4 5 6 7 8; do echo " importance=$i, dl_vlan=$i actions=drop"; 
done | sort > expout
+AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed 
'/OFPST_FLOW/d' | sort], [0], [expout])
+
+dnl Replace some flows in the bridge. Delete flows that are not present in 
replace-flows.txt
+dnl for i in 1 3 5 7; do echo "dl_vlan=$i,importance=`expr $i + 10` 
actions=drop"; done | sort > replace-flows.txt
+
+for i in 1 3 5 7; do echo "dl_vlan=$i,importance=$i actions=drop"; done | sort 
> replace-flows.txt
 AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 replace-flows.txt])
 
 dnl Dump them and compare the dump flows output against the expected output.
-for i in 1 2 3 4 5 6 7 8; do if [[ `expr $i % 2` -eq 1 ]]; then 
importance=`expr $i + 10`; else importance=$i; fi; echo " 
importance=$importance, dl_vlan=$i actions=drop"; done | sort > expout
-AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed 
'/OFPST_FLOW/d' | sort],
-  [0], [expout])
+for i in 1 3 5 7; do echo " importance=$i, dl_vlan=$i actions=drop"; done | 
sort > expout
+AT_CHECK([ovs-ofctl -O OpenFlow14 dump-flows br0 | ofctl_strip | sed 
'/OFPST_FLOW/d' | sort], [0], [expout])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+dnl --tables option added to ovs-ofctl "replace-flows" and "diff-flows".
+dnl This validates whether flows in table IDs specified with --tables
+dnl are getting replaced with "replace-flows" or not by validating
+dnl dump-flows output after replace with the expected output.
+
+AT_SETUP([ovs-ofctl replace-flows with --tables])
+OVS_VSWITCHD_START
+
+dnl Add flows to br0 for tables 1 to 8.
+for i in 1 2 3 4 5 6 7 8; do echo "table=$i,dl_vlan=$i,actions=drop"; done > 
add-flows.txt
+AT_CHECK([ovs-ofctl add-flows br0 add-flows.txt])
+
+dnl Replace flows from tables 1,3,5,7 in the bridge.
+for i in 1 3 5 7; do echo 
"table=$i,ip,nw_dst=192.168.1.$i,dl_vlan=$i,actions=drop"; done > 
replace-flows.txt
+AT_CHECK([ovs-ofctl -O OpenFlow14 replace-flows br0 repla

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

2015-06-25 Thread Gal Sagie
Hello All,

Currently OVN uses centralized ovsdb-server to serve the Northbound and the
Southbound DB to all the local controllers (sitting at each of the compute
nodes).

It is a single point of failure and probably a major bottleneck to the
operation of OVN in scale.
I know there are efforts to make ovsdb-server distributed using Raft, while
i think this is an important effort i believe that open source is about
being open to alternative and choices while reusing other open and reliable
solutions, this is why i want to suggest the following idea:

Design the DB distribution layer in a pluggable manner, doing so, users can
pick alternate distributed DB options that are reliable and have been in
the market for some time which also have performance optimizations. (Of
course that the default plugin will use the ovsdb-server implementation)
I think an important aspect in this regards is that different
environments/setups with different scale needs can have different solutions
that fits them, the ability to choose which back end to use can help in
these scenarios.

If we analyze OVSDB, there are 3 major areas an alternate solution needs to
support:

1) The DB JSON schema itself
 Should be the same between all solutions

2) The OVSDB protocol features
like: monitor (publish-subscribe) / transactions / garbage collection /
locking
   sync-verify (multi writes/reads for same values)

It is important to note that any pluggable solution must support all
these features

3) The connection between the client and the server
 This i believe can be pluggable as long as the messages that are
exchanged (the protocol
 features) are still exchanged

Then the only thing that needs to be modified is basically the client
library which can map
the API's to the client requests depending on the picked solution.

Looking forward hearing your opinions.

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


Re: [ovs-dev] [PATCH 1/2] Committing two issues:

2015-06-25 Thread Garg, Sumit
Hello Ben, no worries.

Still figuring my way around git, github etc. So, please excuse by
missteps (e.g. subject lines).


I've redone (revert + recommit) changes to my fork with a better commit
message.

Will, be sending the patch (via email) & pull request (via github) shortly.

Let's handle the "initialize new row" as a separate activity.
--
Sumit Garg
Extreme Networks
su...@extremenetworks.com
+1 (919) 595-4971







On 6/24/15, 7:40 PM, "Ben Pfaff"  wrote:

>Hi Sumit.  Sorry it took me a while to review this--I'm catching up as
>best I can.
>
>Can I have a Signed-off-by for this commit?  CONTRIBUTING.md says this
>about Signed-off-by:
>
>Signed-off-by: Author Name 
>
>Informally, this indicates that Author Name is the author or
>submitter of a patch and has the authority to submit it under
>the terms of the license.  The formal meaning is to agree to
>the Developer's Certificate of Origin (see below).
>
>If the author and submitter are different, each must sign off.
>If the patch has more than one author, all must sign off.
>
>Signed-off-by: Author Name 
>Signed-off-by: Submitter Name 
>
>and its meaning is:
>
>Developer's Certificate of Origin
>-
>
>To help track the author of a patch as well as the submission chain,
>and be clear that the developer has authority to submit a patch for
>inclusion in openvswitch please sign off your work.  The sign off
>certifies the following:
>
>Developer's Certificate of Origin 1.1
>
>By making a contribution to this project, I certify that:
>
>(a) The contribution was created in whole or in part by me and I
>have the right to submit it under the open source license
>indicated in the file; or
>
>(b) The contribution is based upon previous work that, to the best
>of my knowledge, is covered under an appropriate open source
>license and I have the right under that license to submit that
>work with modifications, whether created in whole or in part
>by me, under the same open source license (unless I am
>permitted to submit under a different license), as indicated
>in the file; or
>
>(c) The contribution was provided directly to me by some other
>person who certified (a), (b) or (c) and I have not modified
>it.
>
>(d) I understand and agree that this project and the contribution
>are public and that a record of the contribution (including all
>personal information I submit with it, including my sign-off) is
>maintained indefinitely and may be redistributed consistent with
>this project or the open source license(s) involved.
>
>More below.
>
>On Sat, Jun 20, 2015 at 05:09:54PM -0700, Ben Pfaff wrote:
>> From: Sumit Garg 
>>
>> 1. A bool (has_lock) was being accessed as a function call
>>leading to a runtime exception.
>
>This seems obviously correct, I'll happily commit it when I have a
>signoff.
>
>> 2. When 'alert' was turned off on a column, the code was
>>erroring out when value for that column was being set
>>in a newly inserted row. This is because the row._data
>>was None at this time.
>>
>> A question related to change #2 - should a newly inserted
>> row get automatically intialized to default values? If so,
>> I'm not sure the initialization to defaults is happening
>> and maybe that's why I'm seeing the NULL error. Either way,
>> I don't see an issue with adding the additional check.
>
>I think it would be better to initialize a new row to defaults but the
>code doesn't do that at the moment.  I'm happy to take this change.




DISCLAIMER:
This e-mail and any attachments to it may contain confidential and proprietary 
material and is solely for the use of the intended recipient. Any review, use, 
disclosure, distribution or copying of this transmittal is prohibited except by 
or on behalf of the intended recipient. If you have received this transmittal 
in error, please notify the sender and destroy this e-mail and any attachments 
and all copies, whether electronic or printed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/2] When opening a JSONRPC connection, the health probes are incorrectly getting turned off for connections that need probes.

2015-06-25 Thread Garg, Sumit
As stated in the earlier mail, redoing & resubmitting the changes.
--
Sumit Garg
Extreme Networks
su...@extremenetworks.com
+1 (919) 595-4971







On 6/24/15, 7:41 PM, "Ben Pfaff"  wrote:

>On Sat, Jun 20, 2015 at 05:09:55PM -0700, Ben Pfaff wrote:
>> From: Sumit Garg 
>>
>> In other words, when stream_or_pstream_needs_probes()
>> return non-zero, the probes are gettting disabled as
>> the probe interval is getting set to zero. This leads
>> to incorrect behavior such that probes are:
>>
>>  - not getting turned off for unix: connections
>>  - getting turned off for tcp:/ssl: connections
>>
>> The changes in this commit fix this issue.
>
>Seems obviously correct, I just need a sign-off as for the first patch.
>Thank you!




DISCLAIMER:
This e-mail and any attachments to it may contain confidential and proprietary 
material and is solely for the use of the intended recipient. Any review, use, 
disclosure, distribution or copying of this transmittal is prohibited except by 
or on behalf of the intended recipient. If you have received this transmittal 
in error, please notify the sender and destroy this e-mail and any attachments 
and all copies, whether electronic or printed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 0/2] ovn: Add administrative state to logical ports.

2015-06-25 Thread Russell Bryant
On 06/24/2015 08:30 PM, Ben Pfaff wrote:
> On Tue, Jun 23, 2015 at 03:18:57PM -0400, Russell Bryant wrote:
>> On 06/23/2015 02:22 PM, Russell Bryant wrote:
>>> While working on OpenStack Neutron integration for OVN, I came across a 
>>> small
>>> feature gap.  The Neutron API supports setting a port as administratively 
>>> down.
>>> One way to implement that would be to delete ports from OVN while
>>> administratively down.  However, it seemed to me that it would be nice to 
>>> keep
>>> the list of ports that currently exist in sync between the CMS (Neutron in 
>>> this
>>> case) and OVN.  This patch is a first attempt at supporting this OVN by 
>>> updating
>>> the Pipeline to drop packets to/from a port that is administratively down.
>>>
>>>  [PATCH 1/2] ovn: Add logical port 'enabled' state.
>>>  [PATCH 2/2] ovn: Add get/set-enabled to ovn-nbctl.
>>>
>>>  northd/ovn-northd.c |   14 +---
>>>  ovn-nb.ovsschema|1 
>>>  ovn-nb.xml  |7 ++
>>>  ovn-nbctl.8.xml |   13 +++
>>>  ovn-nbctl.c |   59 
>>> 
>>>  5 files changed, 91 insertions(+), 3 deletions(-)
>>>
>>
>> After some more looking around, it seems the appropriate way to do this
>> is with the equivalent of "ovs-ofctl mod-port".  I'll look into how to
>> do that some more.
> 
> I'm happy enough to have this concept at the OVN level.  I don't want
> Neutron to have to talk to OpenFlow at all, if OVN is in the picture.
> 

I was thinking of propagating it down to OVN_Southbound and having
ovn-controller speak OpenFlow.  If you're happy with this first patch
though, that's certainly good with me.  :-)

Thanks!

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


Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Nithin Raju
> On Jun 24, 2015, at 12:57 PM, Eitan Eliahu  wrote:
> 
> 
> I'm wondering if we could fail this call in the datapath level by examining 
> the DPIF_FP_PROBE bit in the flags.
> If it is too hard we can still live with user mode code change.
> Thanks,
> Eitan

That’s a good suggestion. We can go with Alin’s solution for 2.4 since we 
exactly know what code we will be disabling, and work on the flow validation 
logic for master.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Nithin Raju

>> On Jun 18, 2015, at 6:47 PM, Alin Serdean  
>> wrote:
>> 
>> This patch disables features which are not currently supported in the 
>> windows datapath.
>> 
>> Unfortunately we have to do it in userspace because dpif_probe_feature 
>> is not treated accordingly in the windows the datapath.
>> 
>> I opened the issue to track the feature for later implementations:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvs
>> witch_ovs-2Dissues_issues_85&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-
>> YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=H9ymRGwE
>> SDpjH0qHp5EifPK6QimZgzvHEIyjM3QzRwg&s=-rOo7BG2BoxcGi2BlSQASDz8v5Hkhp4y
>> MoGbPZ-zQus&e=

IMO, this patch is good for 2.4 branch. For master, we’ll work on getting 
dpif_probe_feature() to work correctly.

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


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

2015-06-25 Thread Russell Bryant
On 06/25/2015 07:21 AM, Gal Sagie wrote:
> Hello All,
> 
> Currently OVN uses centralized ovsdb-server to serve the Northbound and the
> Southbound DB to all the local controllers (sitting at each of the compute
> nodes).
> 
> It is a single point of failure and probably a major bottleneck to the
> operation of OVN in scale.

Yes, it's definitely something we need to address one way or another.

> I know there are efforts to make ovsdb-server distributed using Raft, while
> i think this is an important effort i believe that open source is about
> being open to alternative and choices while reusing other open and reliable
> solutions, this is why i want to suggest the following idea:
> 
> Design the DB distribution layer in a pluggable manner, doing so, users can
> pick alternate distributed DB options that are reliable and have been in
> the market for some time which also have performance optimizations. (Of
> course that the default plugin will use the ovsdb-server implementation)
> I think an important aspect in this regards is that different
> environments/setups with different scale needs can have different solutions
> that fits them, the ability to choose which back end to use can help in
> these scenarios.
> 
> If we analyze OVSDB, there are 3 major areas an alternate solution needs to
> support:
> 
> 1) The DB JSON schema itself
>  Should be the same between all solutions
> 
> 2) The OVSDB protocol features
> like: monitor (publish-subscribe) / transactions / garbage collection /
> locking
>sync-verify (multi writes/reads for same values)
> 
> It is important to note that any pluggable solution must support all
> these features
> 
> 3) The connection between the client and the server
>  This i believe can be pluggable as long as the messages that are
> exchanged (the protocol
>  features) are still exchanged
> 
> Then the only thing that needs to be modified is basically the client
> library which can map
> the API's to the client requests depending on the picked solution.
> 
> Looking forward hearing your opinions.

Do you have a particular alternative db in mind?  Just curious.  I think
it'd be easier for me to think through it with a specific example in mind.

One way I've thought about approaching playing with this is to write a
ovsdb-to- proxy that would run local to each ovn-controller.
Of course, I'm sure it's harder than it seems in my head.  :-)  It's
probably a waste of time since I don't think anyone would want to use
that for real.

In any case, I do like the idea of still being able to use ovsdb-server
even if we started adding support for something else.  ovsdb-server
keeps the deployment really simple since you already have it for OVS.

I think some abstraction here would be nice.  Even if ovsdb-server
becomes a distributed db, I suspect there would be a difficult
perception and trust battle.  "Ugh, they implemented their own
distributed db?!  NIH, etc .."  "Ugh, a distributed db that nobody else
uses?  I don't trust that."  Those are the reactions I'm already hearing
talking to people about this stuff.

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


Re: [ovs-dev] [PATCH] dpif-netdev: log port/core affinity

2015-06-25 Thread Flavio Leitner
On Tue, Jun 09, 2015 at 03:49:18PM +0100, Mark Kavanagh wrote:
> When using multiple PMDs and numerous ports, a performance gain
> may be achieved in some use cases by pinning a PMD/port to a
> particular (set of) core(s).
> 
> This patch provides a summary of the switch's port/core affinities
> each time that the status of the switch's ports is modified.
> Based on this information, a user may determine what affinity
> modifications are required to optimise performance for their
> particular use case.
> 
> Signed-off-by: Mark Kavanagh 
> Signed-off-by: Wojciech Andralojc 
> ---

That's useful info in my opinion.

Acked-by: Flavio Leitner 


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


Re: [ovs-dev] [PATCH] tests: Automatically add pthread-win32 directory to $PATH.

2015-06-25 Thread Alin Serdean
I tested it and there are two problems with it:

-   PTHREAD_WIN32_DIR is not the actual location of the DLLs the actual 
path would be of the form
   PTHREAD_WIN32_DIR_DLL=$withval/dll/x86

-   $withval/lib/x86(PTHREAD_WIN32_DIR) is of the windows form: 
C:/pthread/dll/x86, while mingw
   expects it to be of the form /C/pthread/dll/x86

Alin.

-Mesaj original-
De la: dev [mailto:dev-boun...@openvswitch.org] În numele Ben Pfaff
Trimis: Thursday, June 25, 2015 5:14 AM
Către: dev@openvswitch.org
Cc: Ben Pfaff
Subiect: [ovs-dev] [PATCH] tests: Automatically add pthread-win32 directory to 
$PATH.

This reduces the user burden for running "make check".

Signed-off-by: Ben Pfaff 
---
I can't test this; whoever reviews it ought to.

 m4/openvswitch.m4 | 6 --
 tests/automake.mk | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index f89cde0..57df79e 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -86,9 +86,11 @@ AC_DEFUN([OVS_CHECK_WIN32],
 AC_MSG_ERROR([Invalid --with-pthread value])
   ;;
 *)
-PTHREAD_INCLUDES="-I$withval/include"
-PTHREAD_LDFLAGS="-L$withval/lib/x86"
+PTHREAD_WIN32_DIR=$withval/lib/x86
+PTHREAD_INCLUDES=-I$withval/include
+PTHREAD_LDFLAGS=-L$PTHREAD_WIN32_DIR
 PTHREAD_LIBS="-lpthreadVC2"
+AC_SUBST([PTHREAD_WIN32_DIR])
 AC_SUBST([PTHREAD_INCLUDES])
 AC_SUBST([PTHREAD_LDFLAGS])
 AC_SUBST([PTHREAD_LIBS])
diff --git a/tests/automake.mk b/tests/automake.mk index 3f57114..714bc91 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -95,7 +95,7 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch  
KMOD_TESTSUITE = $(srcdir)/tests/kmod-testsuite  DISTCLEANFILES += 
tests/atconfig tests/atlocal
 
-AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests
+AUTOTEST_PATH = 
+utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR)
 
 check-local: tests/atconfig tests/atlocal $(TESTSUITE)
$(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH) 
$(TESTSUITEFLAGS)
--
2.1.3

___
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] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Alin Serdean
Both are hacks IMO. 

We could respond to DPIF_FP_PROBE but I prefer to do it in userspace
until we address the bigger problem in the datapath.

Alin.

-Mesaj original-
De la: Nithin Raju [mailto:nit...@vmware.com] 
Trimis: Thursday, June 25, 2015 4:51 PM
Către: Eitan Eliahu
Cc: Ben Pfaff; Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath

> On Jun 24, 2015, at 12:57 PM, Eitan Eliahu  wrote:
> 
> 
> I'm wondering if we could fail this call in the datapath level by examining 
> the DPIF_FP_PROBE bit in the flags.
> If it is too hard we can still live with user mode code change.
> Thanks,
> Eitan

That’s a good suggestion. We can go with Alin’s solution for 2.4 since we 
exactly know what code we will be disabling, and work on the flow validation 
logic for master.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/3] Add OVS_VSWITCHD_STOP to bfd unit tests

2015-06-25 Thread Alin Serdean
Hi Ben,

I am ok with the concept and I will test it if you send a patch out.

I tried to apply the diff but it gives me:
error: patch failed: tests/ovs-macros.at:37
error: tests/ovs-macros.at: patch does not apply

Alin.

-Mesaj original-
De la: Ben Pfaff [mailto:b...@nicira.com] 
Trimis: Thursday, June 25, 2015 3:17 AM
Către: Gurucharan Shetty
Cc: Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH 1/3] Add OVS_VSWITCHD_STOP to bfd unit tests

On Wed, Jun 24, 2015 at 07:49:08AM -0700, Gurucharan Shetty wrote:
> You can look at "commit 5e65e080ad4d57e" for more information on the 
> exact problem with taskkill.
> 
> One option is to check for tskill and if it does not exist, use taskkill.

Here's a concept for that, it's probably buggy because I don't have a handy 
Windows system for testing.  (The minimal change would be to just add the new 
bit at the beginning but I couldn't resist "improving" the rest of the code.)

diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index c583c3d..7427de0 
100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -37,37 +37,31 @@ if test "$IS_WIN32" = "yes"; then
 command diff --strip-trailing-cr "$@"
 }
 
+# tskill is more effective than taskkill but it isn't always installed.
+if (tskill //?) >/dev/null 2>&1; then :; else
+tskill () { taskkill //F //PID $1 >/dev/null; }
+fi
+
 kill () {
-case "$1" in
--0)
-shift
-for i in $*; do
-# tasklist will always have return code 0.
-# If pid does exist, there will be a line with the pid.
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-:
-else
-return 1
-fi
-done
-return 0
-;;
--[1-9]*)
-shift
-for i in $*; do
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-tskill $i
-fi
-done
-;;
+   signal=
+   retval=0
+for arg; do
+   case $arg in
+-*) signal=$arg ;;
 [1-9][0-9]*)
-for i in $*; do
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-tskill $i
+   # tasklist always returns 0.
+   # If pid does exist, there will be a line with the pid.
+   if tasklist //fi "PID eq $arg" | grep $arg >/dev/null; then
+   if test "X$signal" != "X-0"; then
+   tskill $arg
 fi
-done
+   else
+retval=1
+   fi
 ;;
-esac
+esac
+done
+   return $retval
 }
 fi
 ]
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] tests: Automatically add pthread-win32 directory to $PATH.

2015-06-25 Thread Ben Pfaff
OK, I'll drop it, though if anyone else wants to pick up and fix it that
would be fine too.

On Thu, Jun 25, 2015 at 02:26:36PM +, Alin Serdean wrote:
> I tested it and there are two problems with it:
> 
> - PTHREAD_WIN32_DIR is not the actual location of the DLLs the actual 
> path would be of the form
>PTHREAD_WIN32_DIR_DLL=$withval/dll/x86
> 
> - $withval/lib/x86(PTHREAD_WIN32_DIR) is of the windows form: 
> C:/pthread/dll/x86, while mingw
>expects it to be of the form /C/pthread/dll/x86
> 
> Alin.
> 
> -Mesaj original-
> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Ben Pfaff
> Trimis: Thursday, June 25, 2015 5:14 AM
> Către: dev@openvswitch.org
> Cc: Ben Pfaff
> Subiect: [ovs-dev] [PATCH] tests: Automatically add pthread-win32 directory 
> to $PATH.
> 
> This reduces the user burden for running "make check".
> 
> Signed-off-by: Ben Pfaff 
> ---
> I can't test this; whoever reviews it ought to.
> 
>  m4/openvswitch.m4 | 6 --
>  tests/automake.mk | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index f89cde0..57df79e 
> 100644
> --- a/m4/openvswitch.m4
> +++ b/m4/openvswitch.m4
> @@ -86,9 +86,11 @@ AC_DEFUN([OVS_CHECK_WIN32],
>  AC_MSG_ERROR([Invalid --with-pthread value])
>;;
>  *)
> -PTHREAD_INCLUDES="-I$withval/include"
> -PTHREAD_LDFLAGS="-L$withval/lib/x86"
> +PTHREAD_WIN32_DIR=$withval/lib/x86
> +PTHREAD_INCLUDES=-I$withval/include
> +PTHREAD_LDFLAGS=-L$PTHREAD_WIN32_DIR
>  PTHREAD_LIBS="-lpthreadVC2"
> +AC_SUBST([PTHREAD_WIN32_DIR])
>  AC_SUBST([PTHREAD_INCLUDES])
>  AC_SUBST([PTHREAD_LDFLAGS])
>  AC_SUBST([PTHREAD_LIBS])
> diff --git a/tests/automake.mk b/tests/automake.mk index 3f57114..714bc91 
> 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -95,7 +95,7 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch  
> KMOD_TESTSUITE = $(srcdir)/tests/kmod-testsuite  DISTCLEANFILES += 
> tests/atconfig tests/atlocal
>  
> -AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests
> +AUTOTEST_PATH = 
> +utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR)
>  
>  check-local: tests/atconfig tests/atlocal $(TESTSUITE)
>   $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH) 
> $(TESTSUITEFLAGS)
> --
> 2.1.3
> 
> ___
> 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] tests: Automatically add pthread-win32 directory to $PATH.

2015-06-25 Thread Alin Serdean
I'll fix it and also test it. 

Mind if I add you as co-author?

Alin.

-Mesaj original-
De la: Ben Pfaff [mailto:b...@nicira.com] 
Trimis: Thursday, June 25, 2015 5:35 PM
Către: Alin Serdean
Cc: dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH] tests: Automatically add pthread-win32 directory 
to $PATH.

OK, I'll drop it, though if anyone else wants to pick up and fix it that would 
be fine too.

On Thu, Jun 25, 2015 at 02:26:36PM +, Alin Serdean wrote:
> I tested it and there are two problems with it:
> 
> - PTHREAD_WIN32_DIR is not the actual location of the DLLs the actual 
> path would be of the form
>PTHREAD_WIN32_DIR_DLL=$withval/dll/x86
> 
> - $withval/lib/x86(PTHREAD_WIN32_DIR) is of the windows form: 
> C:/pthread/dll/x86, while mingw
>expects it to be of the form /C/pthread/dll/x86
> 
> Alin.
> 
> -Mesaj original-
> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Ben Pfaff
> Trimis: Thursday, June 25, 2015 5:14 AM
> Către: dev@openvswitch.org
> Cc: Ben Pfaff
> Subiect: [ovs-dev] [PATCH] tests: Automatically add pthread-win32 directory 
> to $PATH.
> 
> This reduces the user burden for running "make check".
> 
> Signed-off-by: Ben Pfaff 
> ---
> I can't test this; whoever reviews it ought to.
> 
>  m4/openvswitch.m4 | 6 --
>  tests/automake.mk | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index 
> f89cde0..57df79e 100644
> --- a/m4/openvswitch.m4
> +++ b/m4/openvswitch.m4
> @@ -86,9 +86,11 @@ AC_DEFUN([OVS_CHECK_WIN32],
>  AC_MSG_ERROR([Invalid --with-pthread value])
>;;
>  *)
> -PTHREAD_INCLUDES="-I$withval/include"
> -PTHREAD_LDFLAGS="-L$withval/lib/x86"
> +PTHREAD_WIN32_DIR=$withval/lib/x86
> +PTHREAD_INCLUDES=-I$withval/include
> +PTHREAD_LDFLAGS=-L$PTHREAD_WIN32_DIR
>  PTHREAD_LIBS="-lpthreadVC2"
> +AC_SUBST([PTHREAD_WIN32_DIR])
>  AC_SUBST([PTHREAD_INCLUDES])
>  AC_SUBST([PTHREAD_LDFLAGS])
>  AC_SUBST([PTHREAD_LIBS])
> diff --git a/tests/automake.mk b/tests/automake.mk index 
> 3f57114..714bc91 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -95,7 +95,7 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch  
> KMOD_TESTSUITE = $(srcdir)/tests/kmod-testsuite  DISTCLEANFILES += 
> tests/atconfig tests/atlocal
>  
> -AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests
> +AUTOTEST_PATH =
> +utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR)
>  
>  check-local: tests/atconfig tests/atlocal $(TESTSUITE)
>   $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH) 
> $(TESTSUITEFLAGS)
> --
> 2.1.3
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] acinclude: Require libfuse only for DPDK with vhost-cuse.

2015-06-25 Thread Daniele Di Proietto
DPDK with vhost-user doesn't require libfuse, so we shouldn't link OVS
with libfuse unless DPDK is built with vhost-cuse support.

CC: Rapelly, Varun 
Signed-off-by: Daniele Di Proietto 
---
 acinclude.m4 | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 20391ec..14907ab 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -173,7 +173,11 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 DPDK_INCLUDE=$RTE_SDK/include
 DPDK_LIB_DIR=$RTE_SDK/lib
 DPDK_LIB="-lintel_dpdk"
-DPDK_EXTRA_LIB="-lfuse"
+DPDK_EXTRA_LIB=""
+
+OVS_GREP_IFELSE([$RTE_SDK/include/rte_config.h], [define 
RTE_LIBRTE_VHOST_USER 1],
+[], [AC_DEFINE([VHOST_CUSE], [1], [DPDK vhost-cuse support 
enabled, vhost-user disabled.])
+ DPDK_EXTRA_LIB="-lfuse"])
 
 ovs_save_CFLAGS="$CFLAGS"
 ovs_save_LDFLAGS="$LDFLAGS"
@@ -221,8 +225,6 @@ AC_DEFUN([OVS_CHECK_DPDK], [
 AC_SUBST([DPDK_vswitchd_LDFLAGS])
 AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.])
 
-OVS_GREP_IFELSE([$RTE_SDK/include/rte_config.h], [define 
RTE_LIBRTE_VHOST_USER 1],
-[], [AC_DEFINE([VHOST_CUSE], [1], [DPDK vhost-cuse support 
enabled, vhost-user disabled.])])
   else
 RTE_SDK=
   fi
-- 
2.1.4

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


Re: [ovs-dev] [PATCH] tests: Automatically add pthread-win32 directory to $PATH.

2015-06-25 Thread Ben Pfaff
That's fine.
On Jun 25, 2015 7:42 AM, "Alin Serdean" 
wrote:

> I'll fix it and also test it.
>
> Mind if I add you as co-author?
>
> Alin.
>
> -Mesaj original-
> De la: Ben Pfaff [mailto:b...@nicira.com]
> Trimis: Thursday, June 25, 2015 5:35 PM
> Către: Alin Serdean
> Cc: dev@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] tests: Automatically add pthread-win32
> directory to $PATH.
>
> OK, I'll drop it, though if anyone else wants to pick up and fix it that
> would be fine too.
>
> On Thu, Jun 25, 2015 at 02:26:36PM +, Alin Serdean wrote:
> > I tested it and there are two problems with it:
> >
> > - PTHREAD_WIN32_DIR is not the actual location of the DLLs the
> actual path would be of the form
> >PTHREAD_WIN32_DIR_DLL=$withval/dll/x86
> >
> > - $withval/lib/x86(PTHREAD_WIN32_DIR) is of the windows form:
> C:/pthread/dll/x86, while mingw
> >expects it to be of the form /C/pthread/dll/x86
> >
> > Alin.
> >
> > -Mesaj original-
> > De la: dev [mailto:dev-boun...@openvswitch.org] În numele Ben Pfaff
> > Trimis: Thursday, June 25, 2015 5:14 AM
> > Către: dev@openvswitch.org
> > Cc: Ben Pfaff
> > Subiect: [ovs-dev] [PATCH] tests: Automatically add pthread-win32
> directory to $PATH.
> >
> > This reduces the user burden for running "make check".
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> > I can't test this; whoever reviews it ought to.
> >
> >  m4/openvswitch.m4 | 6 --
> >  tests/automake.mk | 2 +-
> >  2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index
> > f89cde0..57df79e 100644
> > --- a/m4/openvswitch.m4
> > +++ b/m4/openvswitch.m4
> > @@ -86,9 +86,11 @@ AC_DEFUN([OVS_CHECK_WIN32],
> >  AC_MSG_ERROR([Invalid --with-pthread value])
> >;;
> >  *)
> > -PTHREAD_INCLUDES="-I$withval/include"
> > -PTHREAD_LDFLAGS="-L$withval/lib/x86"
> > +PTHREAD_WIN32_DIR=$withval/lib/x86
> > +PTHREAD_INCLUDES=-I$withval/include
> > +PTHREAD_LDFLAGS=-L$PTHREAD_WIN32_DIR
> >  PTHREAD_LIBS="-lpthreadVC2"
> > +AC_SUBST([PTHREAD_WIN32_DIR])
> >  AC_SUBST([PTHREAD_INCLUDES])
> >  AC_SUBST([PTHREAD_LDFLAGS])
> >  AC_SUBST([PTHREAD_LIBS])
> > diff --git a/tests/automake.mk b/tests/automake.mk index
> > 3f57114..714bc91 100644
> > --- a/tests/automake.mk
> > +++ b/tests/automake.mk
> > @@ -95,7 +95,7 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
> > KMOD_TESTSUITE = $(srcdir)/tests/kmod-testsuite  DISTCLEANFILES +=
> > tests/atconfig tests/atlocal
> >
> > -AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests
> > +AUTOTEST_PATH =
> > +utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR)
> >
> >  check-local: tests/atconfig tests/atlocal $(TESTSUITE)
> >   $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH)
> > $(TESTSUITEFLAGS)
> > --
> > 2.1.3
> >
> > ___
> > 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] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Eitan Eliahu


Alin,
We just need to add:
[OVS_FLOW_ATTR_PROBE] = { .type = NLA_FLAG },
to the flow policy. This is not a hack.
If it is time consuming please go ahead with the uder mode change.
Thanks,
Eitan


-Original Message-
From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com] 
Sent: Thursday, June 25, 2015 7:30 AM
To: Nithin Raju; Eitan Eliahu
Cc: Ben Pfaff; dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath

Both are hacks IMO. 

We could respond to DPIF_FP_PROBE but I prefer to do it in userspace until we 
address the bigger problem in the datapath.

Alin.

-Mesaj original-
De la: Nithin Raju [mailto:nit...@vmware.com]
Trimis: Thursday, June 25, 2015 4:51 PM
Către: Eitan Eliahu
Cc: Ben Pfaff; Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath

> On Jun 24, 2015, at 12:57 PM, Eitan Eliahu  wrote:
> 
> 
> I'm wondering if we could fail this call in the datapath level by examining 
> the DPIF_FP_PROBE bit in the flags.
> If it is too hard we can still live with user mode code change.
> Thanks,
> Eitan

That’s a good suggestion. We can go with Alin’s solution for 2.4 since we 
exactly know what code we will be disabling, and work on the flow validation 
logic for master.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 0/2] ovn: Add administrative state to logical ports.

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 09:25:36AM -0400, Russell Bryant wrote:
> On 06/24/2015 08:30 PM, Ben Pfaff wrote:
> > On Tue, Jun 23, 2015 at 03:18:57PM -0400, Russell Bryant wrote:
> >> On 06/23/2015 02:22 PM, Russell Bryant wrote:
> >>> While working on OpenStack Neutron integration for OVN, I came across a 
> >>> small
> >>> feature gap.  The Neutron API supports setting a port as administratively 
> >>> down.
> >>> One way to implement that would be to delete ports from OVN while
> >>> administratively down.  However, it seemed to me that it would be nice to 
> >>> keep
> >>> the list of ports that currently exist in sync between the CMS (Neutron 
> >>> in this
> >>> case) and OVN.  This patch is a first attempt at supporting this OVN by 
> >>> updating
> >>> the Pipeline to drop packets to/from a port that is administratively down.
> >>>
> >>>  [PATCH 1/2] ovn: Add logical port 'enabled' state.
> >>>  [PATCH 2/2] ovn: Add get/set-enabled to ovn-nbctl.
> >>>
> >>>  northd/ovn-northd.c |   14 +---
> >>>  ovn-nb.ovsschema|1 
> >>>  ovn-nb.xml  |7 ++
> >>>  ovn-nbctl.8.xml |   13 +++
> >>>  ovn-nbctl.c |   59 
> >>> 
> >>>  5 files changed, 91 insertions(+), 3 deletions(-)
> >>>
> >>
> >> After some more looking around, it seems the appropriate way to do this
> >> is with the equivalent of "ovs-ofctl mod-port".  I'll look into how to
> >> do that some more.
> > 
> > I'm happy enough to have this concept at the OVN level.  I don't want
> > Neutron to have to talk to OpenFlow at all, if OVN is in the picture.
> > 
> 
> I was thinking of propagating it down to OVN_Southbound and having
> ovn-controller speak OpenFlow.  If you're happy with this first patch
> though, that's certainly good with me.  :-)

I think it's OK, we can always propagate it down later if it turns out
to be useful (I'm not sure whether it is).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH]: bfd: display last wall clock time of last flap

2015-06-25 Thread Ben Pfaff
On Wed, Jun 24, 2015 at 06:08:02PM -0700, Sabyasachi Sengupta wrote:
> >It's a little unconventional for us to use a wall clock time for this.
> >I'd be more inclined to report it as "N seconds ago" or "N ms ago".  Any
> >particular reason to use a wall clock time?
> 
> I've seen that all BFD other outputs use "now -/+" convention, but just that
> I thought wall clock time was more user readable, especially because last
> flap could be hrs/days ago. If we imagine that this output (last flap time)
> could be parsed through a remote script as to which link went down and when,
> its probably easier for them to see the absolute time rather than having to
> convert it (note that the notion of 'now' could be different in both
> machines?). I'd think that it makes more sense for next/last TX times
> (should continue to) be in milliseconds as that reflects a more ongoing
> activity.

It's much easier for a script to parse a number than a date.

I doubt that anyone in real life cares whether the last flap was
yesterday or a week ago.  If the output says 32 seconds, I look for
problems; if it says 138923 seconds, that's no big deal.

> I understand that this will be based on what ovs-vswitchd 'thinks' as to
> when the last flap occurred. Its still not the accurate information as
> ovs-vswitchd might itself have restarted in between the last flap and when
> the user actually reads it. This probably means that we save this info in
> ovsdb and read it through a separate CLI, but probably we can keep it simple
> for now?

I don't think there's a need to store it.  I imagine that restarting OVS
often causes a flap anyway.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] This reduces the user burden for running "make check".

2015-06-25 Thread Alin Serdean
Co-authored-by: Ben Pfaff 
Signed-off-by: Ben Pfaff 
Signed-off-by: Alin Gabriel Serdean 
---
 m4/openvswitch.m4 | 7 +--
 tests/automake.mk | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index f89cde0..087c7e5 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -86,9 +86,12 @@ AC_DEFUN([OVS_CHECK_WIN32],
 AC_MSG_ERROR([Invalid --with-pthread value])
   ;;
 *)
-PTHREAD_INCLUDES="-I$withval/include"
-PTHREAD_LDFLAGS="-L$withval/lib/x86"
+PTHREAD_WIN32_DIR=$withval/lib/x86
+PTHREAD_WIN32_DIR_DLL=/${withval/:/}/dll/x86
+PTHREAD_INCLUDES=-I$withval/include
+PTHREAD_LDFLAGS=-L$PTHREAD_WIN32_DIR
 PTHREAD_LIBS="-lpthreadVC2"
+AC_SUBST([PTHREAD_WIN32_DIR_DLL])
 AC_SUBST([PTHREAD_INCLUDES])
 AC_SUBST([PTHREAD_LDFLAGS])
 AC_SUBST([PTHREAD_LIBS])
diff --git a/tests/automake.mk b/tests/automake.mk
index 3f57114..153d4e1 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -95,7 +95,7 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
 KMOD_TESTSUITE = $(srcdir)/tests/kmod-testsuite
 DISTCLEANFILES += tests/atconfig tests/atlocal
 
-AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests
+AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR_DLL)
 
 check-local: tests/atconfig tests/atlocal $(TESTSUITE)
$(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH) 
$(TESTSUITEFLAGS)
-- 
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] tests: Use taskkill if tskill is not available, on Windows.

2015-06-25 Thread Ben Pfaff
This is not the minimal change; it "improves" the rest of the code as well.

Signed-off-by: Ben Pfaff 
---
Needs testing.

 tests/ovs-macros.at | 48 +---
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index c583c3d..d550204 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -37,37 +37,31 @@ if test "$IS_WIN32" = "yes"; then
 command diff --strip-trailing-cr "$@"
 }
 
+# tskill is more effective than taskkill but it isn't always installed.
+if (tskill //?) >/dev/null 2>&1; then :; else
+tskill () { taskkill //F //PID $1 >/dev/null; }
+fi
+
 kill () {
-case "$1" in
--0)
-shift
-for i in $*; do
-# tasklist will always have return code 0.
-# If pid does exist, there will be a line with the pid.
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-:
-else
-return 1
-fi
-done
-return 0
-;;
--[1-9]*)
-shift
-for i in $*; do
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-tskill $i
-fi
-done
-;;
+signal=
+retval=0
+for arg; do
+case $arg in
+-*) signal=$arg ;;
 [1-9][0-9]*)
-for i in $*; do
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-tskill $i
+# tasklist always returns 0.
+# If pid does exist, there will be a line with the pid.
+if tasklist //fi "PID eq $arg" | grep $arg >/dev/null; then
+if test "X$signal" != "X-0"; then
+tskill $arg
 fi
-done
+else
+retval=1
+fi
 ;;
-esac
+esac
+done
+return $retval
 }
 fi
 ]
-- 
2.1.3

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


Re: [ovs-dev] [PATCH 1/3] Add OVS_VSWITCHD_STOP to bfd unit tests

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 02:33:33PM +, Alin Serdean wrote:
> I am ok with the concept and I will test it if you send a patch out.

OK, here you go:
http://openvswitch.org/pipermail/dev/2015-June/056859.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Alin Serdean
Eitan,

Ok I will add it in the datapath.

Why I am considering it as a hack, because if we see it in the datapath we just 
reply with an error, there will be no logic behind it at the moment. But maybe 
it makes some sense to have it for further user.

Alin.

-Mesaj original-
De la: Eitan Eliahu [mailto:elia...@vmware.com] 
Trimis: Thursday, June 25, 2015 6:01 PM
Către: Alin Serdean; Nithin Raju
Cc: Ben Pfaff; dev@openvswitch.org
Subiect: RE: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath



Alin,
We just need to add:
[OVS_FLOW_ATTR_PROBE] = { .type = NLA_FLAG }, to the flow policy. This 
is not a hack.
If it is time consuming please go ahead with the uder mode change.
Thanks,
Eitan


-Original Message-
From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com]
Sent: Thursday, June 25, 2015 7:30 AM
To: Nithin Raju; Eitan Eliahu
Cc: Ben Pfaff; dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath

Both are hacks IMO. 

We could respond to DPIF_FP_PROBE but I prefer to do it in userspace until we 
address the bigger problem in the datapath.

Alin.

-Mesaj original-
De la: Nithin Raju [mailto:nit...@vmware.com]
Trimis: Thursday, June 25, 2015 4:51 PM
Către: Eitan Eliahu
Cc: Ben Pfaff; Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath

> On Jun 24, 2015, at 12:57 PM, Eitan Eliahu  wrote:
> 
> 
> I'm wondering if we could fail this call in the datapath level by examining 
> the DPIF_FP_PROBE bit in the flags.
> If it is too hard we can still live with user mode code change.
> Thanks,
> Eitan

That’s a good suggestion. We can go with Alin’s solution for 2.4 since we 
exactly know what code we will be disabling, and work on the flow validation 
logic for master.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] This reduces the user burden for running "make check".

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 03:30:34PM +, Alin Serdean wrote:
> Co-authored-by: Ben Pfaff 
> Signed-off-by: Ben Pfaff 
> Signed-off-by: Alin Gabriel Serdean 

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


Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Ben Pfaff
On Fri, Jun 19, 2015 at 01:47:16AM +, Alin Serdean wrote:
> This patch disables features which are not currently supported
> in the windows datapath.
> 
> Unfortunately we have to do it in userspace because dpif_probe_feature is
> not treated accordingly in the windows the datapath.
> 
> I opened the issue to track the feature for later implementations:
> https://github.com/openvswitch/ovs-issues/issues/85

Seems reasonable for 2.4, can I have a sign-off?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/2] python: Fix exceptions in IDL

2015-06-25 Thread Garg, Sumit
Committing two fixes that address python exceptions:

1. A bool (has_lock) was being accessed as a function call
   leading to a runtime exception.

2. When 'alert' was turned off on a column, the code was
   erroring out when value for that column was being set
   in a newly inserted row. This is because the row._data
   was None at this time.

An observation related to change #2 - it seems that new
rows are not initialized to defaults and that's why the
NULL error happens.

IMO a newly inserted row should automatically get
intialized to default values. This new behavior can be
implemented as a separate improvement sometime in the
future.

For now, I don't see an issue with adding the additional
check. This new check can continue as-is even after the
new behavior is implemented.

Signed-off-by: Sumit Garg 
---
 python/ovs/db/idl.py |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 45a5a23..f074dbf 100644

--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -875,7 +875,7 @@ class Transaction(object):
 return self._status

 # If we need a lock but don't have it, give up quickly.
-if self.idl.lock_name and not self.idl.has_lock():
+if self.idl.lock_name and not self.idl.has_lock:
 self._status = Transaction.NOT_LOCKED
 self.__disassemble()
 return self._status
@@ -1074,7 +1074,7 @@ class Transaction(object):
 # transaction only does writes of existing values, without making
any
 # real changes, we will drop the whole transaction later in
 # ovsdb_idl_txn_commit().)
-if not column.alert and row._data.get(column.name) == datum:
+if not column.alert and row._data and row._data.get(column.name)
== datum:
 new_value = row._changes.get(column.name)
 if new_value is None or new_value == datum:
 return
--
1.7.1

--
Sumit Garg
Extreme Networks
su...@extremenetworks.com
+1 (919) 595-4971






DISCLAIMER:
This e-mail and any attachments to it may contain confidential and proprietary 
material and is solely for the use of the intended recipient. Any review, use, 
disclosure, distribution or copying of this transmittal is prohibited except by 
or on behalf of the intended recipient. If you have received this transmittal 
in error, please notify the sender and destroy this e-mail and any attachments 
and all copies, whether electronic or printed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 2/2] python: Fix issue with probes for JSONRPC connections

2015-06-25 Thread Garg, Sumit
When opening a JSONRPC connection, the health probes
are incorrectly getting turned off for connections
that need probes.

In other words, when stream_or_pstream_needs_probes()
return non-zero, the probes are gettting disabled as
the probe interval is getting set to zero. This leads
to incorrect behavior such that probes are:

  - not getting turned off for unix: connections
  - getting turned off for tcp:/ssl: connections

The changes in this commit fix this issue.

Signed-off-by: Sumit Garg 
---
 python/ovs/jsonrpc.py |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/python/ovs/jsonrpc.py b/python/ovs/jsonrpc.py
index 6e7a2cc..d54d74b 100644
--- a/python/ovs/jsonrpc.py
+++ b/python/ovs/jsonrpc.py
@@ -378,7 +378,7 @@ class Session(object):
 if ovs.stream.PassiveStream.is_valid_name(name):
 reconnect.set_passive(True, ovs.timeval.msec())

-if ovs.stream.stream_or_pstream_needs_probes(name):
+if not ovs.stream.stream_or_pstream_needs_probes(name):
 reconnect.set_probe_interval(0)

 return Session(reconnect, None)
--
1.7.1


--
Sumit Garg
Extreme Networks
su...@extremenetworks.com
+1 (919) 595-4971






DISCLAIMER:
This e-mail and any attachments to it may contain confidential and proprietary 
material and is solely for the use of the intended recipient. Any review, use, 
disclosure, distribution or copying of this transmittal is prohibited except by 
or on behalf of the intended recipient. If you have received this transmittal 
in error, please notify the sender and destroy this e-mail and any attachments 
and all copies, whether electronic or printed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] tests: Use taskkill if tskill is not available, on Windows.

2015-06-25 Thread Alin Serdean
Tested with and w/o tskill

Acked-by: Alin Gabriel Serdean 


-Mesaj original-
De la: dev [mailto:dev-boun...@openvswitch.org] În numele Ben Pfaff
Trimis: Thursday, June 25, 2015 6:35 PM
Către: dev@openvswitch.org
Cc: Ben Pfaff
Subiect: [ovs-dev] [PATCH] tests: Use taskkill if tskill is not available, on 
Windows.

This is not the minimal change; it "improves" the rest of the code as well.

Signed-off-by: Ben Pfaff 
---
Needs testing.

 tests/ovs-macros.at | 48 +---
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index c583c3d..d550204 
100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -37,37 +37,31 @@ if test "$IS_WIN32" = "yes"; then
 command diff --strip-trailing-cr "$@"
 }
 
+# tskill is more effective than taskkill but it isn't always installed.
+if (tskill //?) >/dev/null 2>&1; then :; else
+tskill () { taskkill //F //PID $1 >/dev/null; }
+fi
+
 kill () {
-case "$1" in
--0)
-shift
-for i in $*; do
-# tasklist will always have return code 0.
-# If pid does exist, there will be a line with the pid.
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-:
-else
-return 1
-fi
-done
-return 0
-;;
--[1-9]*)
-shift
-for i in $*; do
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-tskill $i
-fi
-done
-;;
+signal=
+retval=0
+for arg; do
+case $arg in
+-*) signal=$arg ;;
 [1-9][0-9]*)
-for i in $*; do
-if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
-tskill $i
+# tasklist always returns 0.
+# If pid does exist, there will be a line with the pid.
+if tasklist //fi "PID eq $arg" | grep $arg >/dev/null; then
+if test "X$signal" != "X-0"; then
+tskill $arg
 fi
-done
+else
+retval=1
+fi
 ;;
-esac
+esac
+done
+return $retval
 }
 fi
 ]
--
2.1.3

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


[ovs-dev] [PATCH] bitmap: Convert single bitmap functions to 64-bit.

2015-06-25 Thread Jesse Gross
Currently the functions to set, clear, and iterate over bitmaps
only operate over 32 bit values. If we convert them to handle
64 bit bitmaps, they can be used in more places.

Suggested-by: Ben Pfaff 
Signed-off-by: Jesse Gross 
---
 lib/bitmap.h  | 10 +-
 lib/cmap.c| 10 +-
 lib/dpif-netdev.c |  6 +++---
 tests/test-cmap.c |  4 ++--
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/lib/bitmap.h b/lib/bitmap.h
index a2ca46e..cf5d3f0 100644
--- a/lib/bitmap.h
+++ b/lib/bitmap.h
@@ -269,13 +269,13 @@ bitmap_is_all_zeros(const unsigned long *bitmap, size_t n)
 #define BITMAP_FOR_EACH_1(IDX, SIZE, BITMAP)\
 BITMAP_FOR_EACH_1_RANGE(IDX, 0, SIZE, BITMAP)
 
-/* More efficient access to a map of single ulong. */
-#define ULONG_FOR_EACH_1(IDX, MAP)  \
-for (unsigned long map__ = (MAP);   \
+/* More efficient access to a map of single ullong. */
+#define ULLONG_FOR_EACH_1(IDX, MAP) \
+for (uint64_t map__ = (MAP);\
  map__ && (((IDX) = raw_ctz(map__)), true); \
  map__ = zero_rightmost_1bit(map__))
 
-#define ULONG_SET0(MAP, OFFSET) ((MAP) &= ~(1UL << (OFFSET)))
-#define ULONG_SET1(MAP, OFFSET) ((MAP) |= 1UL << (OFFSET))
+#define ULLONG_SET0(MAP, OFFSET) ((MAP) &= ~(1ULL << (OFFSET)))
+#define ULLONG_SET1(MAP, OFFSET) ((MAP) |= 1ULL << (OFFSET))
 
 #endif /* bitmap.h */
diff --git a/lib/cmap.c b/lib/cmap.c
index 8d11ae0..7a54ea6 100644
--- a/lib/cmap.c
+++ b/lib/cmap.c
@@ -390,13 +390,13 @@ cmap_find_batch(const struct cmap *cmap, unsigned long 
map,
 uint32_t c1s[sizeof map * CHAR_BIT];
 
 /* Compute hashes and prefetch 1st buckets. */
-ULONG_FOR_EACH_1(i, map) {
+ULLONG_FOR_EACH_1(i, map) {
 h1s[i] = rehash(impl, hashes[i]);
 b1s[i] = &impl->buckets[h1s[i] & impl->mask];
 OVS_PREFETCH(b1s[i]);
 }
 /* Lookups, Round 1. Only look up at the first bucket. */
-ULONG_FOR_EACH_1(i, map) {
+ULLONG_FOR_EACH_1(i, map) {
 uint32_t c1;
 const struct cmap_bucket *b1 = b1s[i];
 const struct cmap_node *node;
@@ -414,12 +414,12 @@ cmap_find_batch(const struct cmap *cmap, unsigned long 
map,
 continue;
 }
 /* Found. */
-ULONG_SET0(map, i); /* Ignore this on round 2. */
+ULLONG_SET0(map, i); /* Ignore this on round 2. */
 OVS_PREFETCH(node);
 nodes[i] = node;
 }
 /* Round 2. Look into the 2nd bucket, if needed. */
-ULONG_FOR_EACH_1(i, map) {
+ULLONG_FOR_EACH_1(i, map) {
 uint32_t c2;
 const struct cmap_bucket *b2 = b2s[i];
 const struct cmap_node *node;
@@ -445,7 +445,7 @@ cmap_find_batch(const struct cmap *cmap, unsigned long map,
 }
 }
 /* Not found. */
-ULONG_SET0(result, i); /* Fix the result. */
+ULLONG_SET0(result, i); /* Fix the result. */
 continue;
 }
 found:
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5009c5f..59f3f14 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3916,14 +3916,14 @@ dpcls_lookup(const struct dpcls *cls, const struct 
netdev_flow_key keys[],
 }
 
 /* Compute hashes for the remaining keys. */
-ULONG_FOR_EACH_1(i, map) {
+ULLONG_FOR_EACH_1(i, map) {
 hashes[i] = netdev_flow_key_hash_in_mask(&mkeys[i],
  &subtable->mask);
 }
 /* Lookup. */
 map = cmap_find_batch(&subtable->rules, map, hashes, nodes);
 /* Check results. */
-ULONG_FOR_EACH_1(i, map) {
+ULLONG_FOR_EACH_1(i, map) {
 struct dpcls_rule *rule;
 
 CMAP_NODE_FOR_EACH (rule, cmap_node, nodes[i]) {
@@ -3932,7 +3932,7 @@ dpcls_lookup(const struct dpcls *cls, const struct 
netdev_flow_key keys[],
 goto next;
 }
 }
-ULONG_SET0(map, i);   /* Did not match. */
+ULLONG_SET0(map, i);  /* Did not match. */
 next:
 ; /* Keep Sparse happy. */
 }
diff --git a/tests/test-cmap.c b/tests/test-cmap.c
index 74037f0..c36ecb6 100644
--- a/tests/test-cmap.c
+++ b/tests/test-cmap.c
@@ -127,7 +127,7 @@ check_cmap(struct cmap *cmap, const int values[], size_t n,
 }
 map = cmap_find_batch(cmap, map, hashes, nodes);
 
-ULONG_FOR_EACH_1(k, map) {
+ULLONG_FOR_EACH_1(k, map) {
 struct element *e;
 
 CMAP_NODE_FOR_EACH (e, node, nodes[k]) {
@@ -435,7 +435,7 @@ find_batch(const struct cmap *cmap, const int value)
 map >>= BITMAP_ULONG_BITS - i; /* Clear excess bits. */
 map = cmap_find_batch(cmap, map, hashes, nodes);
 
-ULONG_FOR_EACH_1(i, map) {
+ULLONG_FOR_EACH_1(i, map) {
 struct element *e;
 
 CMAP

Re: [ovs-dev] [PATCH 10/11] tunnel: Geneve TLV handling support for OpenFlow.

2015-06-25 Thread Jesse Gross
On Wed, Jun 24, 2015 at 7:09 PM, Ben Pfaff  wrote:
> On Wed, Jun 24, 2015 at 06:34:05PM -0700, Jesse Gross wrote:
>> On Wed, Jun 24, 2015 at 3:57 PM, Ben Pfaff  wrote:
>> > On Fri, Jun 19, 2015 at 04:13:24PM -0700, Jesse Gross wrote:
>> > I like the implementation approach used in ULONG_FOR_EACH_1 better than
>> > the one in PRESENT_OPT_FOR_EACH, since the user doesn't have to provide
>> > a scratch variable.  Also it protects the macro arguments better.
>> > Actually ULONG_FOR_EACH_1 could be made to work with 32- or 64-bit maps
>> > just by changing "unsigned long" to "uint64_t" in its definition; maybe
>> > we should.
>>
>> I looked at converting ULONG_FOR_EACH_1 but decided against it since
>> it would introduce an additional branch (to decide whether to use
>> __builtin_ctz or __builtin_ctz_ll)
>
> Really?  It shouldn't, "__builtin_constant_p(n <= UINT32_MAX) && n <=
> UINT32_MAX" should be a compile-time constant.\

You're right, it doesn't. I guess I didn't have enough faith that the
compiler would be able to carry over the max value across an
assignment to a larger variable but I checked and it was fine. I sent
a patch to do it this way instead.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] INSTALL.DPDK: remove experimental statement

2015-06-25 Thread Flavio Leitner
On Tue, Jun 23, 2015 at 06:48:20PM -0700, Pravin Shelar wrote:
> On Tue, Jun 23, 2015 at 11:42 AM, Ben Pfaff  wrote:
> > Do you two have an opinion on this?  If DPDK support is pretty solid now
> > then it makes sense to apply this to master and backport it to
> > branch-2.4.
> 
> Personally I would like to have better testing with vhost, tunneling
> and PMD thread management before removing experimental status. Once
> that is done I will be more comfortable with it.

What kind of test are you looking for?  Indeed, vhost and tunneling
are fairly new, yes, but ethernet ports are being tested for the
whole 2.3 cycle.

My concern is that a new DPDK related feature would push that again.
Perhaps come up with a table stating features X status?

DPDK portsStable
DPDK vhost-cuse   Experimental/Obsolete
DPDK vhost-user   Experimental
DPDK ivshmem  ...
DPDK PMD management   ...

fbl


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


Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Eitan Eliahu
If it gets too involved have your user mode hack instead.
Thanks Alin.
Eitan

-Original Message-
From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com] 
Sent: Thursday, June 25, 2015 8:38 AM
To: Eitan Eliahu; Nithin Raju
Cc: Ben Pfaff; dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath

Eitan,

Ok I will add it in the datapath.

Why I am considering it as a hack, because if we see it in the datapath we just 
reply with an error, there will be no logic behind it at the moment. But maybe 
it makes some sense to have it for further user.

Alin.

-Mesaj original-
De la: Eitan Eliahu [mailto:elia...@vmware.com] 
Trimis: Thursday, June 25, 2015 6:01 PM
Către: Alin Serdean; Nithin Raju
Cc: Ben Pfaff; dev@openvswitch.org
Subiect: RE: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath



Alin,
We just need to add:
[OVS_FLOW_ATTR_PROBE] = { .type = NLA_FLAG }, to the flow policy. This 
is not a hack.
If it is time consuming please go ahead with the uder mode change.
Thanks,
Eitan


-Original Message-
From: Alin Serdean [mailto:aserd...@cloudbasesolutions.com]
Sent: Thursday, June 25, 2015 7:30 AM
To: Nithin Raju; Eitan Eliahu
Cc: Ben Pfaff; dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath

Both are hacks IMO. 

We could respond to DPIF_FP_PROBE but I prefer to do it in userspace until we 
address the bigger problem in the datapath.

Alin.

-Mesaj original-
De la: Nithin Raju [mailto:nit...@vmware.com]
Trimis: Thursday, June 25, 2015 4:51 PM
Către: Eitan Eliahu
Cc: Ben Pfaff; Alin Serdean; dev@openvswitch.org
Subiect: Re: [ovs-dev] [PATCH v2] dpif_probe_feature not suported in windows 
datapath

> On Jun 24, 2015, at 12:57 PM, Eitan Eliahu  wrote:
> 
> 
> I'm wondering if we could fail this call in the datapath level by examining 
> the DPIF_FP_PROBE bit in the flags.
> If it is too hard we can still live with user mode code change.
> Thanks,
> Eitan

That’s a good suggestion. We can go with Alin’s solution for 2.4 since we 
exactly know what code we will be disabling, and work on the flow validation 
logic for master.

thanks,
-- Nithin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Reset RSS hash when receiving from a vhost device.

2015-06-25 Thread Flavio Leitner
On Wed, Jun 24, 2015 at 04:55:40PM +0100, Daniele Di Proietto wrote:
> A vhost device, being a virtual interface, doesn't provide a valid RSS
> hash.  As a workaround we set the value to 0, which suggests the
> datapath to recompute the hash in software.
> 
> Reported-by: Dongjun 
> CC: Traynor, Kevin 
> CC: Flavio Leitner 
> Signed-off-by: Daniele Di Proietto 
> ---

Acked-by: Flavio Leitner 


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


[ovs-dev] [PATCH] [PATCH v3][branch-2.4] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Alin Serdean
This patch disables features which are not currently supported in the
windows datapath.

Unfortunately we have to do it in userspace because dpif_probe_feature is
not treated accordingly in the windows the datapath.

I opened the issue to track the feature for later implementations:
https://github.com/openvswitch/ovs-issues/issues/85

Signed-off-by: Alin Gabriel Serdean 
---
v3: Add Sign-off-by
v2: Rebase
---
---
 ofproto/ofproto-dpif.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 369e0b9..6cdaa32 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1010,8 +1010,14 @@ check_recirc(struct dpif_backer *backer)
 
 ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
 odp_flow_key_from_flow(&key, &flow, NULL, 0, true);
+#ifdef _WIN32
+/* XXX Force disable of datapath recirculation from userspace until the
+ * dpif_probe_feature is properly implemented in the windows datapath */
+enable_recirc = false;
+#else
 enable_recirc = dpif_probe_feature(backer->dpif, "recirculation", &key,
NULL);
+#endif
 
 if (enable_recirc) {
 VLOG_INFO("%s: Datapath supports recirculation",
@@ -1045,7 +1051,13 @@ check_ufid(struct dpif_backer *backer)
 odp_flow_key_from_flow(&key, &flow, NULL, 0, true);
 dpif_flow_hash(backer->dpif, key.data, key.size, &ufid);
 
+#ifdef _WIN32
+/* XXX Force disable of datapath recirculation from userspace until the
+ * dpif_probe_feature is properly implemented in the windows datapath */
+enable_ufid = false;
+#else
 enable_ufid = dpif_probe_feature(backer->dpif, "UFID", &key, &ufid);
+#endif
 
 if (enable_ufid) {
 VLOG_INFO("%s: Datapath supports unique flow ids",
@@ -1151,6 +1163,11 @@ check_max_mpls_depth(struct dpif_backer *backer)
 
 ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
 odp_flow_key_from_flow(&key, &flow, NULL, 0, false);
+#ifdef _WIN32
+/* XXX Force disable of datapath recirculation from userspace until the
+ * dpif_probe_feature is properly implemented in the windows datapath 
*/
+break;
+#endif
 if (!dpif_probe_feature(backer->dpif, "MPLS", &key, NULL)) {
 break;
 }
-- 
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] INSTALL.DPDK: remove experimental statement

2015-06-25 Thread Thomas F Herbert

On 6/25/15 12:39 PM, Flavio Leitner wrote:

On Tue, Jun 23, 2015 at 06:48:20PM -0700, Pravin Shelar wrote:

On Tue, Jun 23, 2015 at 11:42 AM, Ben Pfaff  wrote:

Do you two have an opinion on this?  If DPDK support is pretty solid now
then it makes sense to apply this to master and backport it to
branch-2.4.


Personally I would like to have better testing with vhost, tunneling
and PMD thread management before removing experimental status. Once
that is done I will be more comfortable with it.


What kind of test are you looking for?  Indeed, vhost and tunneling
are fairly new, yes, but ethernet ports are being tested for the
whole 2.3 cycle.

My concern is that a new DPDK related feature would push that again.
Perhaps come up with a table stating features X status?
Flavio, This sounds like a practical approach toward additional testing 
that will help move this forward. +1


DPDK portsStable
DPDK vhost-cuse   Experimental/Obsolete
DPDK vhost-user   Experimental
DPDK ivshmem  ...
DPDK PMD management   ...

fbl


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




--
Thomas F. Herbert
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] [PATCH v3][branch-2.4] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Eitan Eliahu
Acked-by: Eitan Eliahu 
Thanks,
Eitan

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean
Sent: Thursday, June 25, 2015 10:19 AM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] [PATCH v3][branch-2.4] dpif_probe_feature not 
suported in windows datapath

This patch disables features which are not currently supported in the windows 
datapath.

Unfortunately we have to do it in userspace because dpif_probe_feature is not 
treated accordingly in the windows the datapath.

I opened the issue to track the feature for later implementations:
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_85&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=HJQhy35_OR1JNLBJwkqAnyF4JP3rIFj7afqs8puK-5Q&s=DQfR4oXdznIeaYj4XE1I8Np2yJchVsAJQbB2paQyOf8&e=
 

Signed-off-by: Alin Gabriel Serdean 
---
v3: Add Sign-off-by
v2: Rebase
---
---
 ofproto/ofproto-dpif.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 
369e0b9..6cdaa32 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1010,8 +1010,14 @@ check_recirc(struct dpif_backer *backer)
 
 ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
 odp_flow_key_from_flow(&key, &flow, NULL, 0, true);
+#ifdef _WIN32
+/* XXX Force disable of datapath recirculation from userspace until the
+ * dpif_probe_feature is properly implemented in the windows datapath */
+enable_recirc = false;
+#else
 enable_recirc = dpif_probe_feature(backer->dpif, "recirculation", &key,
NULL);
+#endif
 
 if (enable_recirc) {
 VLOG_INFO("%s: Datapath supports recirculation", @@ -1045,7 +1051,13 
@@ check_ufid(struct dpif_backer *backer)
 odp_flow_key_from_flow(&key, &flow, NULL, 0, true);
 dpif_flow_hash(backer->dpif, key.data, key.size, &ufid);
 
+#ifdef _WIN32
+/* XXX Force disable of datapath recirculation from userspace until the
+ * dpif_probe_feature is properly implemented in the windows datapath */
+enable_ufid = false;
+#else
 enable_ufid = dpif_probe_feature(backer->dpif, "UFID", &key, &ufid);
+#endif
 
 if (enable_ufid) {
 VLOG_INFO("%s: Datapath supports unique flow ids", @@ -1151,6 +1163,11 
@@ check_max_mpls_depth(struct dpif_backer *backer)
 
 ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
 odp_flow_key_from_flow(&key, &flow, NULL, 0, false);
+#ifdef _WIN32
+/* XXX Force disable of datapath recirculation from userspace until the
+ * dpif_probe_feature is properly implemented in the windows datapath 
*/
+break;
+#endif
 if (!dpif_probe_feature(backer->dpif, "MPLS", &key, NULL)) {
 break;
 }
--
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=HJQhy35_OR1JNLBJwkqAnyF4JP3rIFj7afqs8puK-5Q&s=qV4ezZ0iFbhztu1BiqvQsHBK0NjZa-WCQ0gQpUm4TvI&e=
 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] [PATCH v3][branch-2.4] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Nithin Raju

> On Jun 25, 2015, at 10:18 AM, Alin Serdean  
> wrote:
> 
> This patch disables features which are not currently supported in the
> windows datapath.
> 
> Unfortunately we have to do it in userspace because dpif_probe_feature is
> not treated accordingly in the windows the datapath.
> 
> I opened the issue to track the feature for later implementations:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_85&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=eqtjKANK7HW1-aE3afNliToXuPx7-T7-qUrCLi9SS4E&s=lniwq8nbDmgchoSl7t952XtbLDU59qAZorSBAK_JHlw&e=
>  
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---


For 2.4 branch:

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


Re: [ovs-dev] [PATCH] tests: Use taskkill if tskill is not available, on Windows.

2015-06-25 Thread Ben Pfaff
Thanks!  I applied this to master and branch-2.4.

On Thu, Jun 25, 2015 at 04:16:14PM +, Alin Serdean wrote:
> Tested with and w/o tskill
> 
> Acked-by: Alin Gabriel Serdean 
> 
> 
> -Mesaj original-
> De la: dev [mailto:dev-boun...@openvswitch.org] În numele Ben Pfaff
> Trimis: Thursday, June 25, 2015 6:35 PM
> Către: dev@openvswitch.org
> Cc: Ben Pfaff
> Subiect: [ovs-dev] [PATCH] tests: Use taskkill if tskill is not available, on 
> Windows.
> 
> This is not the minimal change; it "improves" the rest of the code as well.
> 
> Signed-off-by: Ben Pfaff 
> ---
> Needs testing.
> 
>  tests/ovs-macros.at | 48 +---
>  1 file changed, 21 insertions(+), 27 deletions(-)
> 
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index c583c3d..d550204 
> 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -37,37 +37,31 @@ if test "$IS_WIN32" = "yes"; then
>  command diff --strip-trailing-cr "$@"
>  }
>  
> +# tskill is more effective than taskkill but it isn't always installed.
> +if (tskill //?) >/dev/null 2>&1; then :; else
> +tskill () { taskkill //F //PID $1 >/dev/null; }
> +fi
> +
>  kill () {
> -case "$1" in
> --0)
> -shift
> -for i in $*; do
> -# tasklist will always have return code 0.
> -# If pid does exist, there will be a line with the pid.
> -if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
> -:
> -else
> -return 1
> -fi
> -done
> -return 0
> -;;
> --[1-9]*)
> -shift
> -for i in $*; do
> -if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
> -tskill $i
> -fi
> -done
> -;;
> +signal=
> +retval=0
> +for arg; do
> +case $arg in
> +-*) signal=$arg ;;
>  [1-9][0-9]*)
> -for i in $*; do
> -if tasklist //fi "PID eq $i" | grep $i >/dev/null; then
> -tskill $i
> +# tasklist always returns 0.
> +# If pid does exist, there will be a line with the pid.
> +if tasklist //fi "PID eq $arg" | grep $arg >/dev/null; then
> +if test "X$signal" != "X-0"; then
> +tskill $arg
>  fi
> -done
> +else
> +retval=1
> +fi
>  ;;
> -esac
> +esac
> +done
> +return $retval
>  }
>  fi
>  ]
> --
> 2.1.3
> 
> ___
> 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] bitmap: Convert single bitmap functions to 64-bit.

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 09:22:09AM -0700, Jesse Gross wrote:
> Currently the functions to set, clear, and iterate over bitmaps
> only operate over 32 bit values. If we convert them to handle
> 64 bit bitmaps, they can be used in more places.
> 
> Suggested-by: Ben Pfaff 
> Signed-off-by: Jesse Gross 

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


Re: [ovs-dev] [PATCH 10/11] tunnel: Geneve TLV handling support for OpenFlow.

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 09:24:20AM -0700, Jesse Gross wrote:
> On Wed, Jun 24, 2015 at 7:09 PM, Ben Pfaff  wrote:
> > On Wed, Jun 24, 2015 at 06:34:05PM -0700, Jesse Gross wrote:
> >> On Wed, Jun 24, 2015 at 3:57 PM, Ben Pfaff  wrote:
> >> > On Fri, Jun 19, 2015 at 04:13:24PM -0700, Jesse Gross wrote:
> >> > I like the implementation approach used in ULONG_FOR_EACH_1 better than
> >> > the one in PRESENT_OPT_FOR_EACH, since the user doesn't have to provide
> >> > a scratch variable.  Also it protects the macro arguments better.
> >> > Actually ULONG_FOR_EACH_1 could be made to work with 32- or 64-bit maps
> >> > just by changing "unsigned long" to "uint64_t" in its definition; maybe
> >> > we should.
> >>
> >> I looked at converting ULONG_FOR_EACH_1 but decided against it since
> >> it would introduce an additional branch (to decide whether to use
> >> __builtin_ctz or __builtin_ctz_ll)
> >
> > Really?  It shouldn't, "__builtin_constant_p(n <= UINT32_MAX) && n <=
> > UINT32_MAX" should be a compile-time constant.\
> 
> You're right, it doesn't. I guess I didn't have enough faith that the
> compiler would be able to carry over the max value across an
> assignment to a larger variable but I checked and it was fine. I sent
> a patch to do it this way instead.

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


Re: [ovs-dev] [PATCH] [PATCH v3][branch-2.4] dpif_probe_feature not suported in windows datapath

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 05:18:41PM +, Alin Serdean wrote:
> This patch disables features which are not currently supported in the
> windows datapath.
> 
> Unfortunately we have to do it in userspace because dpif_probe_feature is
> not treated accordingly in the windows the datapath.
> 
> I opened the issue to track the feature for later implementations:
> https://github.com/openvswitch/ovs-issues/issues/85
> 
> Signed-off-by: Alin Gabriel Serdean 
> ---
> v3: Add Sign-off-by
> v2: Rebase

Thanks, I added the acks and applied this to branch-2.4 (only).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/2] python: Fix exceptions in IDL

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 04:07:51PM +, Garg, Sumit wrote:
> Committing two fixes that address python exceptions:
> 
> 1. A bool (has_lock) was being accessed as a function call
>leading to a runtime exception.
> 
> 2. When 'alert' was turned off on a column, the code was
>erroring out when value for that column was being set
>in a newly inserted row. This is because the row._data
>was None at this time.
> 
> An observation related to change #2 - it seems that new
> rows are not initialized to defaults and that's why the
> NULL error happens.
> 
> IMO a newly inserted row should automatically get
> intialized to default values. This new behavior can be
> implemented as a separate improvement sometime in the
> future.
> 
> For now, I don't see an issue with adding the additional
> check. This new check can continue as-is even after the
> new behavior is implemented.
> 
> Signed-off-by: Sumit Garg 

Thanks, I broke this into two commits (one for each fix) and applied
them to master, branch-2.4, and branch-2.3.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/2] python: Fix issue with probes for JSONRPC connections

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 04:07:56PM +, Garg, Sumit wrote:
> When opening a JSONRPC connection, the health probes
> are incorrectly getting turned off for connections
> that need probes.
> 
> In other words, when stream_or_pstream_needs_probes()
> return non-zero, the probes are gettting disabled as
> the probe interval is getting set to zero. This leads
> to incorrect behavior such that probes are:
> 
>   - not getting turned off for unix: connections
>   - getting turned off for tcp:/ssl: connections
> 
> The changes in this commit fix this issue.
> 
> Signed-off-by: Sumit Garg 

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


Re: [ovs-dev] [PATCH] bitmap: Convert single bitmap functions to 64-bit.

2015-06-25 Thread Jesse Gross
On Thu, Jun 25, 2015 at 10:54 AM, Ben Pfaff  wrote:
> On Thu, Jun 25, 2015 at 09:22:09AM -0700, Jesse Gross wrote:
>> Currently the functions to set, clear, and iterate over bitmaps
>> only operate over 32 bit values. If we convert them to handle
>> 64 bit bitmaps, they can be used in more places.
>>
>> Suggested-by: Ben Pfaff 
>> Signed-off-by: Jesse Gross 
>
> Acked-by: Ben Pfaff 

Thanks, applied to master.

With this change, I also rebased the Geneve series on top of it and
applied that too.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] bitmap: Convert single bitmap functions to 64-bit.

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 11:14:16AM -0700, Jesse Gross wrote:
> On Thu, Jun 25, 2015 at 10:54 AM, Ben Pfaff  wrote:
> > On Thu, Jun 25, 2015 at 09:22:09AM -0700, Jesse Gross wrote:
> >> Currently the functions to set, clear, and iterate over bitmaps
> >> only operate over 32 bit values. If we convert them to handle
> >> 64 bit bitmaps, they can be used in more places.
> >>
> >> Suggested-by: Ben Pfaff 
> >> Signed-off-by: Jesse Gross 
> >
> > Acked-by: Ben Pfaff 
> 
> Thanks, applied to master.
> 
> With this change, I also rebased the Geneve series on top of it and
> applied that too.

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


[ovs-dev] [PATCH] tunneling: Userspace datapath support for Geneve options.

2015-06-25 Thread Jesse Gross
Currently the userspace datapath only supports Geneve in a
basic mode - without options - since the rest of userspace
previously didn't support options either. This enables the
userspace datapath to send and receive options as well.

The receive path for extracting the tunnel options isn't entirely
optimal because it does a lookup on the options on a per-packet
basis, rather than per-flow like the kernel does. This is not
as straightforward to do in the userspace datapath since there
is no translation step between packet formats used in packet vs.
flow lookup. This can be optimized in the future and in the
meantime option support is still useful for testing and simulation.

Signed-off-by: Jesse Gross 
---
 datapath/linux/compat/include/linux/openvswitch.h |   2 +-
 lib/netdev-vport.c|  29 +++--
 lib/odp-util.c|  94 ++
 lib/packets.h |   1 +
 lib/tun-metadata.c| 149 ++
 lib/tun-metadata.h|   7 +
 tests/odp.at  |   1 +
 tests/tunnel-push-pop.at  |  21 +++
 8 files changed, 216 insertions(+), 88 deletions(-)

diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index 6cca501..323d158 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -618,7 +618,7 @@ struct ovs_action_hash {
 };
 
 #ifndef __KERNEL__
-#define TNL_PUSH_HEADER_SIZE 128
+#define TNL_PUSH_HEADER_SIZE 512
 
 /*
  * struct ovs_action_push_tnl - %OVS_ACTION_ATTR_TUNNEL_PUSH
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index ea9abf9..0378302 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -1199,6 +1199,7 @@ netdev_geneve_pop_header(struct dp_packet *packet)
 struct flow_tnl *tnl = &md->tunnel;
 struct genevehdr *gnh;
 unsigned int hlen;
+int err;
 
 memset(md, 0, sizeof *md);
 if (GENEVE_BASE_HLEN > dp_packet_size(packet)) {
@@ -1224,12 +1225,6 @@ netdev_geneve_pop_header(struct dp_packet *packet)
 return EINVAL;
 }
 
-if (gnh->opt_len && gnh->critical) {
-VLOG_WARN_RL(&err_rl, "unknown geneve critical options: %"PRIu8" 
bytes\n",
- gnh->opt_len * 4);
-return EINVAL;
-}
-
 if (gnh->proto_type != htons(ETH_TYPE_TEB)) {
 VLOG_WARN_RL(&err_rl, "unknown geneve encapsulated protocol: %#x\n",
  ntohs(gnh->proto_type));
@@ -1240,6 +1235,13 @@ netdev_geneve_pop_header(struct dp_packet *packet)
 tnl->tun_id = htonll(ntohl(get_16aligned_be32(&gnh->vni)) >> 8);
 tnl->flags |= FLOW_TNL_F_KEY;
 
+err = tun_metadata_from_geneve_header(gnh->options, gnh->opt_len * 4,
+  &tnl->metadata);
+if (err) {
+VLOG_WARN_RL(&err_rl, "invalid geneve options");
+return err;
+}
+
 dp_packet_reset_packet(packet, hlen);
 
 return 0;
@@ -1253,6 +1255,8 @@ netdev_geneve_build_header(const struct netdev *netdev,
 struct netdev_vport *dev = netdev_vport_cast(netdev);
 struct netdev_tunnel_config *tnl_cfg;
 struct genevehdr *gnh;
+int opt_len;
+bool crit_opt;
 
 /* XXX: RCUfy tnl_cfg. */
 ovs_mutex_lock(&dev->mutex);
@@ -1260,12 +1264,19 @@ netdev_geneve_build_header(const struct netdev *netdev,
 
 gnh = udp_build_header(tnl_cfg, tnl_flow, data);
 
-gnh->oam = !!(tnl_flow->tunnel.flags & FLOW_TNL_F_OAM);
-gnh->proto_type = htons(ETH_TYPE_TEB);
 put_16aligned_be32(&gnh->vni, htonl(ntohll(tnl_flow->tunnel.tun_id) << 8));
 
 ovs_mutex_unlock(&dev->mutex);
-data->header_len = GENEVE_BASE_HLEN;
+
+opt_len = tun_metadata_to_geneve_header(&tnl_flow->tunnel.metadata,
+gnh->options, &crit_opt);
+
+gnh->opt_len = opt_len / 4;
+gnh->oam = !!(tnl_flow->tunnel.flags & FLOW_TNL_F_OAM);
+gnh->critical = crit_opt ? 1 : 0; 
+gnh->proto_type = htons(ETH_TYPE_TEB);
+
+data->header_len = GENEVE_BASE_HLEN + opt_len;
 data->tnl_type = OVS_VPORT_TYPE_GENEVE;
 return 0;
 }
diff --git a/lib/odp-util.c b/lib/odp-util.c
index efdc651..f811396 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -69,6 +69,17 @@ static void format_odp_key_attr(const struct nlattr *a,
 const struct hmap *portno_names, struct ds *ds,
 bool verbose);
 
+struct geneve_scan {
+struct geneve_opt d[63];
+int len;
+};
+
+static int scan_geneve(const char *s, struct geneve_scan *key,
+   struct geneve_scan *mask);
+static void format_geneve_opts(const struct geneve_opt *opt,
+   const struct geneve_opt *mask, int opts_len,
+   struct ds *, bool verbose);
+
 static struct n

Re: [ovs-dev] [PATCH v2] Do not flush tx queue which is shared among CPUs since it is always flushed

2015-06-25 Thread Pravin Shelar
On Mon, Jun 8, 2015 at 6:13 PM, Wei li  wrote:
> When tx queue is shared among CPUS,the pkts always be flush in 
> 'netdev_dpdk_eth_send'
> So it is unnecessarily for flushing in netdev_dpdk_rxq_recv
> Otherwise tx will be accessed without locking
>
> Signed-off-by: Wei li 
> ---
> v1->v2: fix typo
>
>  lib/netdev-dpdk.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 63243d8..1e0a483 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -892,8 +892,11 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct 
> dp_packet **packets,
>  int nb_rx;
>
>  /* There is only one tx queue for this core.  Do not flush other
> - * queueus. */
> -if (rxq_->queue_id == rte_lcore_id()) {
> + * queues.
> + * Do not flush tx queue which is shared among CPUs
> + * since it is always flushed */
> +if (rxq_->queue_id == rte_lcore_id() &&
> +   OVS_LIKELY(!dev->txq_needs_locking)) {
>  dpdk_queue_flush(dev, rxq_->queue_id);
>  }
>

I pushed patch to branch-2.4 and master.

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


Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix sparse and clang warnings

2015-06-25 Thread Pravin Shelar
On Wed, Jun 24, 2015 at 8:55 AM, Daniele Di Proietto
 wrote:
> Signed-off-by: Daniele Di Proietto 


I pushed patch to branch-2.4 and master.

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


Re: [ovs-dev] [PATCH 2/2] netdev-dpdk: Reset RSS hash when receiving from a vhost device.

2015-06-25 Thread Pravin Shelar
On Wed, Jun 24, 2015 at 8:55 AM, Daniele Di Proietto
 wrote:
> A vhost device, being a virtual interface, doesn't provide a valid RSS
> hash.  As a workaround we set the value to 0, which suggests the
> datapath to recompute the hash in software.
>
> Reported-by: Dongjun 
> CC: Traynor, Kevin 
> CC: Flavio Leitner 
> Signed-off-by: Daniele Di Proietto 


I pushed patch to branch-2.4.
Can you send separate patch for master?

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


Re: [ovs-dev] [PATCH] dpif-netdev: log port/core affinity

2015-06-25 Thread Pravin Shelar
On Tue, Jun 9, 2015 at 7:49 AM, Mark Kavanagh  wrote:
> When using multiple PMDs and numerous ports, a performance gain
> may be achieved in some use cases by pinning a PMD/port to a
> particular (set of) core(s).
>
> This patch provides a summary of the switch's port/core affinities
> each time that the status of the switch's ports is modified.
> Based on this information, a user may determine what affinity
> modifications are required to optimise performance for their
> particular use case.
>
> Signed-off-by: Mark Kavanagh 
> Signed-off-by: Wojciech Andralojc 

I pushed patch to branch-2.4 and master.

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


[ovs-dev] [PATCH] expr: Fix typo in comment.

2015-06-25 Thread Ben Pfaff
Signed-off-by: Ben Pfaff 
---
 ovn/lib/expr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/lib/expr.h b/ovn/lib/expr.h
index 501152e..b411e65 100644
--- a/ovn/lib/expr.h
+++ b/ovn/lib/expr.h
@@ -293,7 +293,7 @@ bool expr_relop_from_token(enum lex_type type, enum 
expr_relop *relop);
  *   have grandchildren of its own type.
  *
  *   As a consequence, every nonterminal node at the same distance from the
- *   root of the root has the same type.
+ *   root has the same type.
  *
  * - EXPR_T_AND and EXPR_T_OR nodes must have at least two children.
  *
-- 
2.1.3

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


Re: [ovs-dev] [PATCH 3/4] ovn-integrate: A new OVN utility script.

2015-06-25 Thread Gurucharan Shetty
On Wed, Jun 24, 2015 at 4:47 PM, Ben Pfaff  wrote:
> On Mon, Jun 22, 2015 at 02:15:49AM -0700, Gurucharan Shetty wrote:
>> Signed-off-by: Gurucharan Shetty 
>
> Is this intended exclusively for use with Docker, or is it meant to be
> more generic?  Is the documentation just the Docker installation
> instructions in the following patch?

All but one of the functions in the newly introduced utility is used as part of
the Docker integration installation instructions with OVN. The one function
(nics-to-bridge) that is not used will be useful for instructions for
a particular use case of
Docker which hasn't yet been added to the instructions (because Docker
does not yet support a particular use case). I do feel the function
(nics-to-bridge) is useful
even when integrating OVN for non-docker uses cases (like initial
configuration in hypervisors).
But I am happy to drop it (if I do drop it, I don't need the first 2
patches of this series committed either)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCHv2 2/2] vlog: add "vlog/list-pattern" command

2015-06-25 Thread Ansis Atteka
This patch allows to query logging format at the runtime for each destination
with "vlog/list-pattern" command.

Signed-Off-By: Ansis Atteka 
---
 NEWS   |  2 ++
 include/openvswitch/vlog.h |  1 +
 lib/vlog-unixctl.man   |  3 +++
 lib/vlog.c | 39 +++
 utilities/ovs-appctl.8.in  |  3 +++
 utilities/ovs-appctl.c |  1 +
 6 files changed, 49 insertions(+)

diff --git a/NEWS b/NEWS
index 2c26a04..e114f5e 100644
--- a/NEWS
+++ b/NEWS
@@ -118,6 +118,8 @@ v2.4.0 - xx xxx 
  See ovs-sim(1) for more information.
- Support to configure method (--syslog-method argument) that determines
  how daemons will talk with syslog.
+   - Support for "ovs-appctl vlog/list-pattern" command that lets to query
+ logging message format for each destination.
 
 
 v2.3.0 - 14 Aug 2014
diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
index f2fedae..f6bb3ab 100644
--- a/include/openvswitch/vlog.h
+++ b/include/openvswitch/vlog.h
@@ -133,6 +133,7 @@ void vlog_set_levels(struct vlog_module *,
 char *vlog_set_levels_from_string(const char *) OVS_WARN_UNUSED_RESULT;
 void vlog_set_levels_from_string_assert(const char *);
 char *vlog_get_levels(void);
+char *vlog_get_patterns(void);
 bool vlog_is_enabled(const struct vlog_module *, enum vlog_level);
 bool vlog_should_drop(const struct vlog_module *, enum vlog_level,
   struct vlog_rate_limit *);
diff --git a/lib/vlog-unixctl.man b/lib/vlog-unixctl.man
index 85dd11d..7c47634 100644
--- a/lib/vlog-unixctl.man
+++ b/lib/vlog-unixctl.man
@@ -51,6 +51,9 @@ Sets the log pattern for \fIdestination\fR to \fIpattern\fR.  
Refer to
 .IP "\fBvlog/list\fR"
 Lists the supported logging modules and their current levels.
 .
+.IP "\fBvlog/list-pattern\fR"
+Lists logging patterns used for each destination.
+.
 .IP "\fBvlog/reopen\fR"
 Causes \fB\*(PN\fR to close and reopen its log file.  (This is useful
 after rotating log files, to cause a new log file to be used.)
diff --git a/lib/vlog.c b/lib/vlog.c
index 359c888..6f1c80c 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -615,6 +615,17 @@ vlog_unixctl_list(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 }
 
 static void
+vlog_unixctl_list_pattern(struct unixctl_conn *conn, int argc,
+  const char *argv[], void *aux OVS_UNUSED)
+{
+char *msg;
+
+msg = vlog_get_patterns();
+unixctl_command_reply(conn, msg);
+free(msg);
+}
+
+static void
 vlog_unixctl_reopen(struct unixctl_conn *conn, int argc OVS_UNUSED,
 const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
 {
@@ -721,6 +732,8 @@ vlog_init(void)
 1, INT_MAX, vlog_unixctl_set, NULL);
 unixctl_command_register("vlog/list", "", 0, 0, vlog_unixctl_list,
  NULL);
+unixctl_command_register("vlog/list-pattern", "", 0, 0,
+ vlog_unixctl_list_pattern, NULL);
 unixctl_command_register("vlog/enable-rate-limit", "[module]...",
  0, INT_MAX, vlog_enable_rate_limit, NULL);
 unixctl_command_register("vlog/disable-rate-limit", "[module]...",
@@ -785,6 +798,32 @@ vlog_get_levels(void)
 return ds_cstr(&s);
 }
 
+/* Returns as a string current logging patterns for each destination.
+ * This string must be released by caller. */
+char *
+vlog_get_patterns(void)
+{
+struct ds ds = DS_EMPTY_INITIALIZER;
+enum vlog_destination destination;
+
+ovs_rwlock_rdlock(&pattern_rwlock);
+ds_put_format(&ds, " prefixformat\n");
+ds_put_format(&ds, " ----\n");
+
+for (destination = 0; destination < VLF_N_DESTINATIONS; destination++) {
+struct destination *f = &destinations[destination];;
+const char *prefix = "none";
+
+if (destination == VLF_SYSLOG && syslogger) {
+prefix = syslog_get_prefix(syslogger);
+}
+ds_put_format(&ds, "%-7s  %-32s  %s\n", f->name, prefix, f->pattern);
+}
+ovs_rwlock_unlock(&pattern_rwlock);
+
+return ds_cstr(&ds);
+}
+
 /* Returns true if a log message emitted for the given 'module' and 'level'
  * would cause some log output, false if that module and level are completely
  * disabled. */
diff --git a/utilities/ovs-appctl.8.in b/utilities/ovs-appctl.8.in
index 9e33f61..ff9c431 100644
--- a/utilities/ovs-appctl.8.in
+++ b/utilities/ovs-appctl.8.in
@@ -112,6 +112,9 @@ and adjusting log levels.
 .IP "\fBvlog/list\fR"
 Lists the known logging modules and their current levels.
 .
+.IP "\fBvlog/list-pattern\fR]"
+Lists logging pattern used for each destination.
+.
 .IP "\fBvlog/set\fR [\fIspec\fR]"
 Sets logging levels.  Without any \fIspec\fR, sets the log level for
 every module and destination to \fBdbg\fR.  Otherwise, \fIspec\fR is a
diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c
index f7403f7..ff6163c 100

[ovs-dev] [PATCHv2 1/2] vlog: abstract out interface to syslog daemon

2015-06-25 Thread Ansis Atteka
This patch helps to address two issues that are present on Ubuntu
15.04 (and most likely other Linux distributions) where rsyslog daemon
is configured to relay log messages from OVS to a remote log collector
and syslog format being used is something other than the one defined in
RFC 3164.  These two issues are:

1. libc syslog() function always adds RFC 3164 prefix to syslog
   messages before sending them over /dev/log Unix domain socket.
   This does not allow us to use libc syslog() function to log in
   RFC 5424 format;  and

2. rsyslogd daemon that comes with Ubuntu 15.04 is too old and
   uses hardcoded syslog message parser when it received messages
   over /dev/log UNIX domain socket.

Solution to those two issues would be to use the newly introduced
--syslog-method=udp:127.0.0.1:514 command line argument when starting
OVS.

Signed-Off-By: Ansis Atteka 
---
 NEWS   |   2 +
 include/openvswitch/vlog.h |   8 
 lib/automake.mk|   5 +++
 lib/syslog-direct.c| 110 +
 lib/syslog-direct.h|  22 +
 lib/syslog-libc.c  |  76 +++
 lib/syslog-libc.h  |  22 +
 lib/syslog-provider.h  |  50 +
 lib/vlog.c |  41 -
 lib/vlog.man   |  25 +++
 10 files changed, 350 insertions(+), 11 deletions(-)
 create mode 100644 lib/syslog-direct.c
 create mode 100644 lib/syslog-direct.h
 create mode 100644 lib/syslog-libc.c
 create mode 100644 lib/syslog-libc.h
 create mode 100644 lib/syslog-provider.h

diff --git a/NEWS b/NEWS
index 395444b..2c26a04 100644
--- a/NEWS
+++ b/NEWS
@@ -116,6 +116,8 @@ v2.4.0 - xx xxx 
- Support for STT tunneling.
- ovs-sim: New developer tool for simulating multiple OVS instances.
  See ovs-sim(1) for more information.
+   - Support to configure method (--syslog-method argument) that determines
+ how daemons will talk with syslog.
 
 
 v2.3.0 - 14 Aug 2014
diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
index 680fba4..f2fedae 100644
--- a/include/openvswitch/vlog.h
+++ b/include/openvswitch/vlog.h
@@ -143,6 +143,9 @@ void vlog_set_pattern(enum vlog_destination, const char 
*pattern);
 int vlog_set_log_file(const char *file_name);
 int vlog_reopen_log_file(void);
 
+/* Configure method how vlog should send messages to syslog server. */
+void vlog_set_syslog_method(const char *method);
+
 /* Configure syslog target. */
 void vlog_set_syslog_target(const char *target);
 
@@ -229,11 +232,13 @@ void vlog_rate_limit(const struct vlog_module *, enum 
vlog_level,
 /* Command line processing. */
 #define VLOG_OPTION_ENUMS   \
 OPT_LOG_FILE,   \
+OPT_SYSLOG_IMPL,\
 OPT_SYSLOG_TARGET
 
 #define VLOG_LONG_OPTIONS   \
 {"verbose",   optional_argument, NULL, 'v'},\
 {"log-file",  optional_argument, NULL, OPT_LOG_FILE},   \
+{"syslog-method", optional_argument, NULL, OPT_SYSLOG_IMPL},\
 {"syslog-target", required_argument, NULL, OPT_SYSLOG_TARGET}
 
 #define VLOG_OPTION_HANDLERS\
@@ -243,6 +248,9 @@ void vlog_rate_limit(const struct vlog_module *, enum 
vlog_level,
 case OPT_LOG_FILE:  \
 vlog_set_log_file(optarg);  \
 break;  \
+case OPT_SYSLOG_IMPL:   \
+vlog_set_syslog_method(optarg); \
+break;  \
 case OPT_SYSLOG_TARGET: \
 vlog_set_syslog_target(optarg); \
 break;
diff --git a/lib/automake.mk b/lib/automake.mk
index 706ff4b..761f3a0 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -237,6 +237,11 @@ lib_libopenvswitch_la_SOURCES = \
lib/string.c \
lib/svec.c \
lib/svec.h \
+   lib/syslog-direct.c \
+   lib/syslog-direct.h \
+   lib/syslog-libc.c \
+   lib/syslog-libc.h \
+   lib/syslog-provider.h \
lib/table.c \
lib/table.h \
lib/tag.c \
diff --git a/lib/syslog-direct.c b/lib/syslog-direct.c
new file mode 100644
index 000..d62c3b5
--- /dev/null
+++ b/lib/syslog-direct.c
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing p

Re: [ovs-dev] [RFC 4/4] Introduce an openvswitch driver for Docker networking.

2015-06-25 Thread Gurucharan Shetty
On Wed, Jun 24, 2015 at 4:48 PM, Ben Pfaff  wrote:
> On Mon, Jun 22, 2015 at 02:15:50AM -0700, Gurucharan Shetty wrote:
>> Docker committed experimental support for multi-host
>> networking yesterday. This commit adds a driver that
>> works with that experimental support. Since Docker
>> code is not part of any official Docker releases yet,
>> this patch is sent as a RFC.
>>
>> Signed-off-by: Gurucharan Shetty 
>
> I don't really feel qualified to review this but I will if you like.

I do think, a quick glance from you would be useful. This won't be
committed right away, but alteast you will know which way this whole
thing is headed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/3] mcast-snooping: Use IPv6 address for MDB

2015-06-25 Thread Flavio Leitner
On Tue, Jun 23, 2015 at 05:03:15PM -0300, Thadeu Lima de Souza Cascardo wrote:
> Use IPv6 internally for storing multicast addresses. IPv4 addresses are
> translated to their IPv4-mapped equivalent.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo 
> ---
>  lib/mcast-snooping.c | 69 
> +++-
>  lib/mcast-snooping.h | 24 +++
>  ofproto/ofproto-dpif-xlate.c |  6 ++--
>  ofproto/ofproto-dpif.c   |  5 ++--
>  4 files changed, 79 insertions(+), 25 deletions(-)
> 
> diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
> index 7b927aa..f2684f3 100644
> --- a/lib/mcast-snooping.c
> +++ b/lib/mcast-snooping.c
> @@ -87,10 +87,11 @@ mcast_bundle_age(const struct mcast_snooping *ms,
>  }
>  
>  static uint32_t
> -mcast_table_hash(const struct mcast_snooping *ms, ovs_be32 grp_ip4,
> - uint16_t vlan)
> +mcast_table_hash(const struct mcast_snooping *ms,
> + const struct in6_addr *grp_addr, uint16_t vlan)
>  {
> -return hash_3words((OVS_FORCE uint32_t) grp_ip4, vlan, ms->secret);
> +return hash_words((const uint32_t *) grp_addr->s6_addr, 4,
> +  hash_2words(ms->secret, vlan));
>  }
>  
>  static struct mcast_group_bundle *
> @@ -108,8 +109,8 @@ mcast_group_from_lru_node(struct ovs_list *list)
>  /* Searches 'ms' for and returns an mcast group for destination address
>   * 'dip' in 'vlan'. */
>  struct mcast_group *
> -mcast_snooping_lookup(const struct mcast_snooping *ms, ovs_be32 dip,
> -  uint16_t vlan)
> +mcast_snooping_lookup(const struct mcast_snooping *ms,
> +  const struct in6_addr *dip, uint16_t vlan)
>  OVS_REQ_RDLOCK(ms->rwlock)
>  {
>  struct mcast_group *grp;
> @@ -117,13 +118,32 @@ mcast_snooping_lookup(const struct mcast_snooping *ms, 
> ovs_be32 dip,
>  
>  hash = mcast_table_hash(ms, dip, vlan);
>  HMAP_FOR_EACH_WITH_HASH (grp, hmap_node, hash, &ms->table) {
> -if (grp->vlan == vlan && grp->ip4 == dip) {
> +if (grp->vlan == vlan && ipv6_addr_equals(&grp->addr, dip)) {
> return grp;
>  }
>  }
>  return NULL;
>  }
>  
> +static inline void
> +in6_addr_set_mapped_ipv4(struct in6_addr *addr, ovs_be32 ip4)
> +{
> +union ovs_16aligned_in6_addr *taddr = (void *) addr;
> +memset(taddr->be16, 0, sizeof(taddr->be16));
> +taddr->be16[5] = 0x;
> +put_16aligned_be32(&taddr->be32[3], ip4);
> +}

I am fine with the ipv4 to ipv6 mapping because it is enforcing
starting with all bits zero, then 16 bits one plus the IPv4 which
is how IPv4 addresses are mapped to IPv6.
[...]

> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 04f6229..cb6d303 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4429,8 +4429,9 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn 
> *conn,
>  bundle = b->port;
>  ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port,
> name, sizeof name);
> -ds_put_format(&ds, "%5s  %4d  "IP_FMT" %3d\n",
> -  name, grp->vlan, IP_ARGS(grp->ip4),
> +ds_put_format(&ds, "%5s  %4d  ", name, grp->vlan);
> +print_ipv6_addr(&ds, &grp->addr);
> +ds_put_format(&ds, " %3d\n",
>mcast_bundle_age(ofproto->ms, b));

But here it exposes the internal implementation showing only IPv6
addresses, so everyone looking at the output would have to known
that their IPv4 addresses are mapped into IPv6 space.

fbl

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


Re: [ovs-dev] [PATCH 3/3] Multicast Listener Discovery support

2015-06-25 Thread Flavio Leitner
On Tue, Jun 23, 2015 at 05:03:16PM -0300, Thadeu Lima de Souza Cascardo wrote:
> Add support for MLDv1 and MLDv2. The behavior is not that different from
> IGMP. Packets to all-hosts address and queries are always flooded,
> reports go to routers, routers are added when a query is observed, and
> all MLD packets go through slow path.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo 
> ---
>  lib/flow.h   | 25 +
>  lib/mcast-snooping.c | 67 ++
>  lib/mcast-snooping.h |  4 +++
>  lib/packets.c|  1 +
>  lib/packets.h| 40 +
>  ofproto/ofproto-dpif-xlate.c | 85 
> +---
>  6 files changed, 209 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/flow.h b/lib/flow.h
> index 70554e4..e68dd74 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -758,6 +758,31 @@ static inline bool is_icmpv6(const struct flow *flow)
>  && flow->nw_proto == IPPROTO_ICMPV6);
>  }
>  
> +static inline bool is_igmp(const struct flow *flow)
> +{
> +return (flow->dl_type == htons(ETH_TYPE_IP)
> +&& flow->nw_proto == IPPROTO_IGMP);
> +}
> +
> +static inline bool is_mld(const struct flow *flow)
> +{
> +return is_icmpv6(flow)
> +   && (flow->tp_src == htons(MLD_QUERY)
> +   || flow->tp_src == htons(MLD_REPORT)
> +   || flow->tp_src == htons(MLD_DONE)
> +   || flow->tp_src == htons(MLD2_REPORT));
> +}
> +
> +static inline bool is_mld_query(const struct flow *flow)
> +{
> +return is_icmpv6(flow) && flow->tp_src == htons(MLD_QUERY);
> +}
> +
> +static inline bool is_mld_report(const struct flow *flow)
> +{
> +return is_mld(flow) && !is_mld_query(flow);
> +}
> +
>  static inline bool is_stp(const struct flow *flow)
>  {
>  return (eth_addr_equals(flow->dl_dst, eth_addr_stp)
> diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
> index f2684f3..aee80b1 100644
> --- a/lib/mcast-snooping.c
> +++ b/lib/mcast-snooping.c
> @@ -499,6 +499,73 @@ mcast_snooping_add_report(struct mcast_snooping *ms,
>  return count;
>  }
>  
> +int
> +mcast_snooping_add_mld(struct mcast_snooping *ms,
> +  const struct dp_packet *p,
> +  uint16_t vlan, void *port)
> +{
> +const struct in6_addr *addr;
> +size_t offset;
> +const struct mld_header *mld;
> +const struct mld2_record *record;
> +int count = 0;
> +int ngrp;
> +bool ret;
> +
> +offset = (char *) dp_packet_l4(p) - (char *) dp_packet_data(p);
> +mld = dp_packet_at(p, offset, MLD_HEADER_LEN);
> +if (!mld) {
> +return 0;
> +}
> +ngrp = ntohs(mld->ngrp);
> +offset += MLD_HEADER_LEN;
> +addr = dp_packet_at(p, offset, sizeof(struct in6_addr));
> +
> +switch (mld->type) {
> +case MLD_REPORT:
> +ret = mcast_snooping_add_group(ms, addr, vlan, port);
> +if (ret)
> +count++;
> +break;
> +case MLD_DONE:
> +ret = mcast_snooping_leave_group(ms, addr, vlan, port);
> +if (ret)
> +count++;
> +break;
> +case MLD2_REPORT:
> +while (ngrp--) {
> +record = dp_packet_at(p, offset, sizeof(struct mld2_record));
> +if (!record) {
> +break;
> +}
> +/* Only consider known record types. */
> +if (record->type < IGMPV3_MODE_IS_INCLUDE
> +|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
> +continue;
> +}
> +addr = &record->maddr;
> +/*
> + * If record is INCLUDE MODE and there are no sources, it's 
> equivalent
> + * to a LEAVE.
> + */
> +if (ntohs(record->nsrcs) == 0
> +&& (record->type == IGMPV3_MODE_IS_INCLUDE
> +|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
> +ret = mcast_snooping_leave_group(ms, addr, vlan, port);
> +} else {
> +ret = mcast_snooping_add_group(ms, addr, vlan, port);
> +}
> +if (ret) {
> +count++;
> +}
> +offset += sizeof(*record)
> +  + ntohs(record->nsrcs) * sizeof(struct in6_addr) + 
> record->aux_len;
> +}
> +}
> +
> +return count;
> +}
> +
>  bool
>  mcast_snooping_leave_group(struct mcast_snooping *ms,
> const struct in6_addr *addr,
> diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
> index e3d15e4..99c314d 100644
> --- a/lib/mcast-snooping.h
> +++ b/lib/mcast-snooping.h
> @@ -194,6 +194,10 @@ int mcast_snooping_add_report(struct mcast_snooping *ms,
>const struct dp_packet *p,
>uint16_t vlan, void *port)
>  OVS_REQ_WRLOCK(ms->rwlock);
> +int mcast_snooping_add_mld(struct mcast_snooping *

Re: [ovs-dev] [PATCH 3/4] ovn-integrate: A new OVN utility script.

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 12:47:56PM -0700, Gurucharan Shetty wrote:
> On Wed, Jun 24, 2015 at 4:47 PM, Ben Pfaff  wrote:
> > On Mon, Jun 22, 2015 at 02:15:49AM -0700, Gurucharan Shetty wrote:
> >> Signed-off-by: Gurucharan Shetty 
> >
> > Is this intended exclusively for use with Docker, or is it meant to be
> > more generic?  Is the documentation just the Docker installation
> > instructions in the following patch?
> 
> All but one of the functions in the newly introduced utility is used as part 
> of
> the Docker integration installation instructions with OVN. The one function
> (nics-to-bridge) that is not used will be useful for instructions for
> a particular use case of
> Docker which hasn't yet been added to the instructions (because Docker
> does not yet support a particular use case). I do feel the function
> (nics-to-bridge) is useful
> even when integrating OVN for non-docker uses cases (like initial
> configuration in hypervisors).
> But I am happy to drop it (if I do drop it, I don't need the first 2
> patches of this series committed either)

I was really heading more in the direction of suggesting a name that
mentioned docker.  I'm OK with including the nics-to-bridge
functionality.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] expr: Fix typo in comment.

2015-06-25 Thread Andy Zhou
Acked-by: Andy Zhou 

On Thu, Jun 25, 2015 at 11:27 AM, Ben Pfaff  wrote:
> Signed-off-by: Ben Pfaff 
> ---
>  ovn/lib/expr.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ovn/lib/expr.h b/ovn/lib/expr.h
> index 501152e..b411e65 100644
> --- a/ovn/lib/expr.h
> +++ b/ovn/lib/expr.h
> @@ -293,7 +293,7 @@ bool expr_relop_from_token(enum lex_type type, enum 
> expr_relop *relop);
>   *   have grandchildren of its own type.
>   *
>   *   As a consequence, every nonterminal node at the same distance from 
> the
> - *   root of the root has the same type.
> + *   root has the same type.
>   *
>   * - EXPR_T_AND and EXPR_T_OR nodes must have at least two children.
>   *
> --
> 2.1.3
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 3/4] ovn-integrate: A new OVN utility script.

2015-06-25 Thread Gurucharan Shetty
> I was really heading more in the direction of suggesting a name that
> mentioned docker.  I'm OK with including the nics-to-bridge
> functionality.

I think it will be true for any other container formats too (if at
all). And it is likely useful to setup VM based OVN solution too.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 3/4] ovn-integrate: A new OVN utility script.

2015-06-25 Thread Ben Pfaff
On Thu, Jun 25, 2015 at 01:51:02PM -0700, Gurucharan Shetty wrote:
> > I was really heading more in the direction of suggesting a name that
> > mentioned docker.  I'm OK with including the nics-to-bridge
> > functionality.
> 
> I think it will be true for any other container formats too (if at
> all). And it is likely useful to setup VM based OVN solution too.

OK, that explains the name then.

I didn't really review the code yet since I was still trying to
understand how it fit in.  I'll review it soon.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] Do not flush tx queue which is shared among CPUs since it is always flushed

2015-06-25 Thread Pravin Shelar
On Wed, Jun 17, 2015 at 3:17 AM, Gray, Mark D  wrote:
>
>
>> -Original Message-
>> From: Daniele Di Proietto [mailto:diproiet...@vmware.com]
>> Sent: Tuesday, June 16, 2015 6:45 PM
>> To: Pravin Shelar; Wei li; Gray, Mark D
>> Cc: d...@openvswitch.com
>> Subject: Re: [ovs-dev] [PATCH v2] Do not flush tx queue which is shared
>> among CPUs since it is always flushed
>>
>>
>>
>> On 16/06/2015 07:40, "Pravin Shelar"  wrote:
>>
>> >On Mon, Jun 8, 2015 at 7:42 PM, Pravin Shelar  wrote:
>> >> On Mon, Jun 8, 2015 at 6:13 PM, Wei li  wrote:
>> >>> When tx queue is shared among CPUS,the pkts always be flush in
>> >>>'netdev_dpdk_eth_send'
>> >>> So it is unnecessarily for flushing in netdev_dpdk_rxq_recv
>> >>>Otherwise tx will be accessed without locking
>> >>>
>> >>> Signed-off-by: Wei li 
>> >>> ---
>> >>> v1->v2: fix typo
>> >>>
>> >>>  lib/netdev-dpdk.c | 7 +--
>> >>>  1 file changed, 5 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>> >>> 63243d8..1e0a483 100644
>> >>> --- a/lib/netdev-dpdk.c
>> >>> +++ b/lib/netdev-dpdk.c
>> >>> @@ -892,8 +892,11 @@ netdev_dpdk_rxq_recv(struct netdev_rxq
>> *rxq_,
>> >>>struct dp_packet **packets,
>> >>>  int nb_rx;
>> >>>
>> >>>  /* There is only one tx queue for this core.  Do not flush other
>> >>> - * queueus. */
>> >>> -if (rxq_->queue_id == rte_lcore_id()) {
>> >>> + * queues.
>> >>> + * Do not flush tx queue which is shared among CPUs
>> >>> + * since it is always flushed */
>> >>> +if (rxq_->queue_id == rte_lcore_id() &&
>> >>> +   OVS_LIKELY(!dev->txq_needs_locking)) {
>> >>>  dpdk_queue_flush(dev, rxq_->queue_id);
>> >>>  }
>> >>
>> >> Patch looks good, But Daniele has plan to get rid of queue flushing
>> >> logic. do you see problem with doing this?
>> >
>> >Daniele,
>> >I am not sure if we should fix this queue flushing logic if we are
>> >going to remove it. So I would like to discuss it first.
>> >You mentioned that the removing flushing logic results in performance
>> >drop. Can you explain it?  How much is performance drop and which is
>> >the case?
>>
>> Hi Pravin,
>>
>> sorry for the late reply.  I suspected that removing the queues in netdev-
>> dpdk was going to have a performance impact in the following scenario: a
>> batch of packets from different megaflows with the same action (output on
>> port 1).
>> Here's what happens:
>>
>> 1) With the queues in netdev-dpdk.  dpif-netdev calls netdev_send() for
>> each
>>packet.  netdev_dpdk_send() just copies the pointer in the queue. Before
>>receiving the next batch dpdk_queue_flush() call rte_eth_tx_burst() with
>>a whole batch of packets
>> 2) Without the queues in netdev-dpdk. dpif-netdev calls netdev_send() for
>> each
>>packet.  netdev_dpdk_send() calls rte_eth_tx_burst() for each packet.
>>
>> Scenario 2 could be slower because rte_eth_tx_burst() is called for each
>> packet (instead of each batch). After some testing this turns out not to be
>> the case.
>> It appears that the bottleneck is not rte_eth_tx_burst(), and copying the
>> pointers in the temporary queue makes the code slower.
>>
>> What do you think?  Should we just remove the queues?
>> CC'ing Mark since he expressed interest in the issue
>
> Every call to rte_eth_tx_burst() will generate an expensive MMIO write. Best
> practice would be to burst as many packets as you can to amortize the cost
> of the MMIO write. I am surprised at your findings for (2) above. Maybe in 
> this
> case the cost of queuing is greater than the MMIO write?
>

Hi Mark,
Can you explain your test case where you do see performance flow down
if there is no queuing in netdev-dpdk?

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


[ovs-dev] [join ovsdb schema v2 0/8] ovsdb-tools changes

2015-06-25 Thread Andy Zhou
OVSDB can be more flexible by supporting multiple schemas. Those multiple 
schemas can be taken
from different releases, or on different feature sets. If the schema tables 
don't overlap,
or for tables do overlap, but columns are not in conflict, those schemas can be 
joined together
into single schema for database operations.

I am working towards convert all ovsdb related tools and servers to support 
multiple schemas.
This patch series is the first attempt, which adds support for ovsdb-tool. This 
will enable
more work of other tools and scripts.

v1-v2:
  I have reworked the original 5 patch series in now 8 patch series, so it 
is harder to
  track review changes on per patch basis. Here is a summary of changes:

  * Add more comments
  * API changes to make it more obvious where memory allocation and release 
should
happen.
  * Add ovsdb-tool man page changes

Andy Zhou (8):
  ovsdb: add functions to support joining multiple schemas
  ovsdb: add more multiple schemas support functions
  ovsdb: fix joined the schema name
  ovsdb: add schemas into ovsdb run time data structure
  ovsdb: enhance file interface to deal with schemas
  ovsdb: add logic for parsing schema file names from a string
  ovsdb-tool: add multiple schemas support
  tests: add ovsdb-tool join schema tests

 ovsdb/column.c|  16 ++-
 ovsdb/column.h|   4 +-
 ovsdb/execution.c |   4 +-
 ovsdb/file.c  |  72 -
 ovsdb/file.h  |  11 +-
 ovsdb/ovsdb-tool.1.in |  77 +++--
 ovsdb/ovsdb-tool.c| 193 +
 ovsdb/ovsdb.c | 292 +-
 ovsdb/ovsdb.h |  57 +++---
 ovsdb/table.c |  39 +++
 ovsdb/table.h |   3 +
 tests/ovsdb-tool.at   | 153 ++
 tests/test-ovsdb.c|   6 +-
 13 files changed, 796 insertions(+), 131 deletions(-)

-- 
1.9.1

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


[ovs-dev] [join ovsdb schema v2 1/8] ovsdb: add functions to support joining multiple schemas

2015-06-25 Thread Andy Zhou
Compatible schemas can be joined together by merging their tables
and columns. Schemas are compatible if they don't contain tables
or columns that are conflict in types.

Signed-off-by: Andy Zhou 
---
 ovsdb/column.c | 16 +-
 ovsdb/column.h |  4 +++-
 ovsdb/ovsdb.c  | 68 +-
 ovsdb/ovsdb.h  |  6 +-
 ovsdb/table.c  | 39 +
 ovsdb/table.h  |  3 +++
 6 files changed, 132 insertions(+), 4 deletions(-)

diff --git a/ovsdb/column.c b/ovsdb/column.c
index 26b7a0b..007ff9b 100644
--- a/ovsdb/column.c
+++ b/ovsdb/column.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -53,6 +53,20 @@ ovsdb_column_clone(const struct ovsdb_column *old)
&old->type);
 }
 
+bool
+ovsdb_column_equal(const struct ovsdb_column *a,
+   const struct ovsdb_column *b)
+{
+struct json *ja = ovsdb_column_to_json(a);
+struct json *jb = ovsdb_column_to_json(b);
+bool equals = json_equal(ja, jb);
+
+json_destroy(ja);
+json_destroy(jb);
+
+return equals;
+}
+
 void
 ovsdb_column_destroy(struct ovsdb_column *column)
 {
diff --git a/ovsdb/column.h b/ovsdb/column.h
index f75a107..a5f67b9 100644
--- a/ovsdb/column.h
+++ b/ovsdb/column.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -48,6 +48,8 @@ struct ovsdb_column *ovsdb_column_create(
 const char *name, bool mutable, bool persistent,
 const struct ovsdb_type *);
 struct ovsdb_column *ovsdb_column_clone(const struct ovsdb_column *);
+bool ovsdb_column_equal(const struct ovsdb_column *,
+const struct ovsdb_column *);
 void ovsdb_column_destroy(struct ovsdb_column *);
 
 struct ovsdb_error *ovsdb_column_from_json(const struct json *,
diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c
index 56d2333..4a2353f 100644
--- a/ovsdb/ovsdb.c
+++ b/ovsdb/ovsdb.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -74,6 +74,35 @@ ovsdb_schema_destroy(struct ovsdb_schema *schema)
 free(schema);
 }
 
+/* Join two schemas into a single schema, by adding missing tables from 'src'
+ * to 'dst'. In case 'src' and 'dst' both has the same table, the joined 'dst' 
table
+ * will contain the columns from both 'src' and 'dst' tables.  */
+static struct ovsdb_error *
+ovsdb_schema_join(struct ovsdb_schema *dst, const struct ovsdb_schema *src)
+{
+struct shash_node *snode;
+
+SHASH_FOR_EACH (snode, &src->tables) {
+const struct ovsdb_table_schema *sts = snode->data;
+struct shash_node *dnode;
+
+dnode = shash_find(&dst->tables, sts->name);
+if (dnode) {
+struct ovsdb_table_schema *dts = dnode->data;
+struct ovsdb_error *err;
+
+err = ovsdb_table_schema_join(dts, sts);
+if (err) {
+return err;
+}
+} else {
+shash_add(&dst->tables, sts->name, ovsdb_table_schema_clone(sts));
+}
+}
+
+return NULL;
+}
+
 struct ovsdb_error *
 ovsdb_schema_from_file(const char *file_name, struct ovsdb_schema **schemap)
 {
@@ -429,3 +458,40 @@ ovsdb_remove_replica(struct ovsdb *db OVS_UNUSED, struct 
ovsdb_replica *r)
 list_remove(&r->node);
 (r->class->destroy)(r);
 }
+
+/* Join each schema within 'schemas' into a single joined schema.
+ * 'schemas' passed in should have at least one schema.
+ *
+ * On success, this function returns NULL, 'schemap' points to the newly 
created
+ * joined schema. The caller is responsible for reclaiming memory via
+ * ovsdb_schema_destroy().
+ *
+ * On error, an ovsdb_error will be allocated describing the error, the caller
+ * is only needs to dispose memory of the error. */
+struct ovsdb_error *
+ovsdb_schemas_join(const struct shash *schemas, struct ovsdb_schema **schemap)
+{
+struct shash_node *first_node = shash_first(schemas), *node;
+struct ovsdb_schema *schema;
+
+ovs_assert(first_node);
+schema = ovsdb_schema_clone(first_node->data);
+
+SHASH_FOR_EACH (node, schemas) {
+struct ovsdb_error *error;
+
+if (node == first_node) {
+continue;
+}
+
+error = ovsdb_schema_join(schema, node->data);
+if (error) {
+ovsdb_schema_destroy(schema);
+*schemap = NULL;
+return error;
+}
+}
+
+*schem

[ovs-dev] [join ovsdb schema v2 2/8] ovsdb: add more multiple schemas support functions

2015-06-25 Thread Andy Zhou
Add more support functions for handling schemas as a set (shash,
actually). They will be used in future patches.

Signed-off-by: Andy Zhou 
---
 ovsdb/ovsdb.c | 164 ++
 ovsdb/ovsdb.h |  33 
 2 files changed, 197 insertions(+)

diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c
index 4a2353f..9e430ce 100644
--- a/ovsdb/ovsdb.c
+++ b/ovsdb/ovsdb.c
@@ -18,6 +18,7 @@
 #include "ovsdb.h"
 
 #include "column.h"
+#include "dynamic-string.h"
 #include "json.h"
 #include "ovsdb-error.h"
 #include "ovsdb-parser.h"
@@ -495,3 +496,166 @@ ovsdb_schemas_join(const struct shash *schemas, struct 
ovsdb_schema **schemap)
 *schemap = schema;
 return NULL;
 }
+
+void
+ovsdb_schemas_destroy(struct shash *schemas)
+{
+struct shash_node *node, *next;
+
+SHASH_FOR_EACH_SAFE (node, next, schemas) {
+struct ovsdb_schema *schema = node->data;
+
+shash_delete(schemas, node);
+ovsdb_schema_destroy(schema);
+}
+shash_destroy(schemas);
+free(schemas);
+}
+
+struct shash *
+ovsdb_schemas_clone(const struct shash *schemas)
+{
+struct shash_node *node;
+struct shash *new_schemas;
+
+new_schemas = xmalloc(sizeof *new_schemas);
+shash_init(new_schemas);
+
+SHASH_FOR_EACH (node, schemas) {
+struct ovsdb_schema *schema = node->data;
+
+shash_add(new_schemas, schema->name, ovsdb_schema_clone(schema));
+}
+
+return new_schemas;
+
+}
+
+struct ovsdb_error *
+ovsdb_schemas_from_files(const struct sset *files, struct shash **schemasp)
+{
+struct ovsdb_error *err;
+struct shash *schemas;
+const char *filename;
+
+ovs_assert(schemasp);
+
+schemas = xmalloc(sizeof *schemas);
+shash_init(schemas);
+
+SSET_FOR_EACH(filename, files) {
+struct ovsdb_schema *schema;
+
+err = ovsdb_schema_from_file(filename, &schema);
+if (err) {
+goto error;
+};
+
+if (shash_find(schemas, schema->name)) {
+err = ovsdb_error("schemas read", "duplicated schema names");
+   ovsdb_schema_destroy(schema);
+goto error;
+};
+
+shash_add(schemas, schema->name, schema);
+}
+
+*schemasp = schemas;
+return NULL;
+
+error:
+ovsdb_schemas_destroy(schemas);
+*schemasp = NULL;
+
+return err;
+}
+
+struct json *
+ovsdb_schemas_to_json(const struct shash *schemas)
+{
+switch (shash_count(schemas)) {
+case 0:
+return NULL;
+
+case 1:
+{
+struct shash_node *node;
+struct ovsdb_schema *schema;
+
+node = shash_first(schemas);
+schema = node->data;
+
+return ovsdb_schema_to_json(schema);
+}
+
+default:
+{
+struct json *json_schemas;
+struct shash_node *node;
+
+json_schemas = json_array_create_empty();
+SHASH_FOR_EACH (node, schemas) {
+struct ovsdb_schema *schema = node->data;
+json_array_add(json_schemas, ovsdb_schema_to_json(schema));
+}
+return json_schemas;
+}
+}
+}
+
+struct ovsdb_error *
+ovsdb_schemas_from_json(struct json *json, struct shash **schemasp)
+{
+struct ovsdb_schema *schema;
+struct ovsdb_error *err;
+struct shash *schemas;
+
+schemas = xmalloc(sizeof *schemas);
+shash_init(schemas);
+
+switch (json->type) {
+case JSON_OBJECT:
+err = ovsdb_schema_from_json(json, &schema);
+if (err) {
+goto error;
+}
+shash_add(schemas, schema->name, schema);
+break;
+
+case JSON_ARRAY:
+{
+struct json_array *array = json_array(json);
+size_t i;
+
+for(i = 0; i < array->n; i++) {
+err = ovsdb_schema_from_json(array->elems[i], &schema);
+
+if (err) {
+goto error;
+}
+shash_add(schemas, schema->name, schema);
+}
+}
+break;
+
+case JSON_NULL:
+case JSON_FALSE:
+case JSON_TRUE:
+case JSON_INTEGER:
+case JSON_REAL:
+case JSON_STRING:
+case JSON_N_TYPES:
+default:
+OVS_NOT_REACHED();
+}
+
+*schemasp = schemas;
+return NULL;
+
+error:
+ovsdb_schemas_destroy(schemas);
+free(schemas);
+*schemasp = NULL;
+
+return err;
+}
diff --git a/ovsdb/ovsdb.h b/ovsdb/ovsdb.h
index 4faad20..771ff41 100644
--- a/ovsdb/ovsdb.h
+++ b/ovsdb/ovsdb.h
@@ -20,6 +20,7 @@
 #include "hmap.h"
 #include "list.h"
 #include "shash.h"
+#include "sset.h"
 
 struct json;
 struct ovsdb_log;
@@ -50,6 +51,38 @@ struct ovsdb_error *ovsdb_schema_from_json(struct json *,
 OVS_WARN_UNUSED_RESULT;
 struct json *ovsdb_schema_to_json(const struct ovsdb_schema *);
 
+/* Multiple schemas. */
+struct shash *ovsdb_schemas_clone(const struct shash *schemas);
+struct ovsdb_error *ovsdb_schemas_from_files(const struct s

[ovs-dev] [join ovsdb schema v2 3/8] ovsdb: fix joined the schema name

2015-06-25 Thread Andy Zhou
The joined schema name currently is taken from any one of the schemas,
(actually, it is taken from the first schema on the shash). This won't
work since transactions needs to verify schema name.

This patch makes the joined schema name as a comma separated string
of names from the original schemas. As long as an operation is against
one of the original schemas, it is allowed to proceed.

Signed-off-by: Andy Zhou 
---
using strstr() for checking is going to be error prone in the long run,
but it should be sufficient for the goal of this patch:
make ovsdb-tool multi-schema ready. I will revisit this if the
schema name concatenation approach looks O.K.
---
 ovsdb/execution.c |  4 ++--
 ovsdb/ovsdb.c | 12 +++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index de25a87..57fc432 100644
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -106,7 +106,7 @@ ovsdb_execute(struct ovsdb *db, const struct ovsdb_session 
*session,
 if (params->type != JSON_ARRAY
 || !params->u.array.n
 || params->u.array.elems[0]->type != JSON_STRING
-|| strcmp(params->u.array.elems[0]->u.string, db->schema->name)) {
+|| !strstr(db->schema->name, params->u.array.elems[0]->u.string)) {
 if (params->type != JSON_ARRAY) {
 error = ovsdb_syntax_error(params, NULL, "array expected");
 } else {
diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c
index 9e430ce..39e6103 100644
--- a/ovsdb/ovsdb.c
+++ b/ovsdb/ovsdb.c
@@ -77,11 +77,21 @@ ovsdb_schema_destroy(struct ovsdb_schema *schema)
 
 /* Join two schemas into a single schema, by adding missing tables from 'src'
  * to 'dst'. In case 'src' and 'dst' both has the same table, the joined 'dst' 
table
- * will contain the columns from both 'src' and 'dst' tables.  */
+ * will contain the columns from both 'src' and 'dst' tables. 
+ * 
+ * The joined schema name will be the concatenation of each schema's name,
+ * sperated by a comma and a space.  */
 static struct ovsdb_error *
 ovsdb_schema_join(struct ovsdb_schema *dst, const struct ovsdb_schema *src)
 {
 struct shash_node *snode;
+struct ds ds;
+
+/* Combine schema names into a comma seperated string.  */
+ds_init(&ds);
+ds_put_format(&ds, "%s, %s", dst->name, src->name);
+free(dst->name);
+dst->name = ds_steal_cstr(&ds);
 
 SHASH_FOR_EACH (snode, &src->tables) {
 const struct ovsdb_table_schema *sts = snode->data;
-- 
1.9.1

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


[ovs-dev] [join ovsdb schema v2 4/8] ovsdb: add schemas into ovsdb run time data structure

2015-06-25 Thread Andy Zhou
It turns out ovsdb in memory data structure needs to store both
joined schema and the original schemas, so that it can recreate
DB file laster.

This patch adds 'schemas' to 'struct ovsdb' and fixes corresponding
APIs for maintaining this information.

Signed-off-by: Andy Zhou 
---
 ovsdb/file.c   | 3 ++-
 ovsdb/ovsdb.c  | 6 +-
 ovsdb/ovsdb.h  | 9 +
 tests/test-ovsdb.c | 6 +++---
 4 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 8c3c31b..033d6f9 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -199,7 +199,8 @@ ovsdb_file_open__(const char *file_name,
 goto error;
 }
 
-db = ovsdb_create(schema ? schema : ovsdb_schema_clone(alternate_schema));
+db = ovsdb_create(schema ? schema : ovsdb_schema_clone(alternate_schema),
+  NULL);
 
 n_transactions = 0;
 while ((error = ovsdb_log_read(log, &json)) == NULL && json) {
diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c
index 39e6103..5929306 100644
--- a/ovsdb/ovsdb.c
+++ b/ovsdb/ovsdb.c
@@ -361,13 +361,14 @@ ovsdb_set_ref_table(const struct shash *tables,
 }
 
 struct ovsdb *
-ovsdb_create(struct ovsdb_schema *schema)
+ovsdb_create(struct ovsdb_schema *schema, struct shash *schemas)
 {
 struct shash_node *node;
 struct ovsdb *db;
 
 db = xmalloc(sizeof *db);
 db->schema = schema;
+db->schemas = schemas;
 list_init(&db->replicas);
 list_init(&db->triggers);
 db->run_triggers = false;
@@ -421,6 +422,9 @@ ovsdb_destroy(struct ovsdb *db)
 shash_clear(&db->schema->tables);
 
 ovsdb_schema_destroy(db->schema);
+if (db->schemas) {
+ovsdb_schemas_destroy(db->schemas);
+}
 free(db);
 }
 }
diff --git a/ovsdb/ovsdb.h b/ovsdb/ovsdb.h
index 771ff41..2e7f7e0 100644
--- a/ovsdb/ovsdb.h
+++ b/ovsdb/ovsdb.h
@@ -92,16 +92,17 @@ struct ovsdb_error *ovsdb_schemas_join(const struct shash 
*schemas,
 
 /* Database. */
 struct ovsdb {
-struct ovsdb_schema *schema;
-struct ovs_list replicas;   /* Contains "struct ovsdb_replica"s. */
-struct shash tables;/* Contains "struct ovsdb_table *"s. */
+struct ovsdb_schema *schema; /* The joined schema for run time.  */
+struct shash *schemas;   /* The schemas stored in the DB file. */
+struct ovs_list replicas;/* Contains "struct ovsdb_replica"s. */
+struct shash tables; /* Contains "struct ovsdb_table *"s. */
 
 /* Triggers. */
 struct ovs_list triggers;   /* Contains "struct ovsdb_trigger"s. */
 bool run_triggers;
 };
 
-struct ovsdb *ovsdb_create(struct ovsdb_schema *);
+struct ovsdb *ovsdb_create(struct ovsdb_schema *schema, struct shash *schemas);
 void ovsdb_destroy(struct ovsdb *);
 
 void ovsdb_get_memory_usage(const struct ovsdb *, struct simap *usage);
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 7913763..58a5fb7 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1253,7 +1253,7 @@ do_execute(struct ovs_cmdl_context *ctx)
 json = parse_json(ctx->argv[1]);
 check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
 json_destroy(json);
-db = ovsdb_create(schema);
+db = ovsdb_create(schema, NULL);
 
 for (i = 2; i < ctx->argc; i++) {
 struct json *params, *result;
@@ -1307,7 +1307,7 @@ do_trigger(struct ovs_cmdl_context *ctx)
 json = parse_json(ctx->argv[1]);
 check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
 json_destroy(json);
-db = ovsdb_create(schema);
+db = ovsdb_create(schema, NULL);
 
 ovsdb_server_init(&server);
 ovsdb_server_add_db(&server, db);
@@ -1533,7 +1533,7 @@ do_transact(struct ovs_cmdl_context *ctx)
   "   \"j\": {\"type\": \"integer\"}");
 check_ovsdb_error(ovsdb_schema_from_json(json, &schema));
 json_destroy(json);
-do_transact_db = ovsdb_create(schema);
+do_transact_db = ovsdb_create(schema, NULL);
 do_transact_table = ovsdb_get_table(do_transact_db, "mytable");
 assert(do_transact_table != NULL);
 
-- 
1.9.1

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


[ovs-dev] [join ovsdb schema v2 5/8] ovsdb: enhance file interface to deal with schemas

2015-06-25 Thread Andy Zhou
When storing joined schemas into a database file, A JSON array will
be store as its first record, with every individual schema as one
element of the array. To maintain backward compatibility, when
joined schemas contain a single schema, a single JSON object will be
stored, not a JSON array with a single element.

The joined schema is not stored into the DB file, since it can be
rebuilt at run time by rejoining stored schemas mentioned above.

DB file read operations now aceept both multiple schemas
(a JSON array as the first entry) or a single schmea (a JSON object).

This change is fully backward compatible as long as each DB contains
a single schema.

Signed-off-by: Andy Zhou 
---
 ovsdb/file.c | 104 ++-
 ovsdb/file.h |  10 ++
 2 files changed, 91 insertions(+), 23 deletions(-)

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 033d6f9..7709047 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -64,7 +64,7 @@ static struct ovsdb_error *ovsdb_file_txn_commit(struct json 
*,
  struct ovsdb_log *);
 
 static struct ovsdb_error *ovsdb_file_open__(const char *file_name,
- const struct ovsdb_schema *,
+ const struct shash *,
  bool read_only, struct ovsdb **,
  struct ovsdb_file **);
 static struct ovsdb_error *ovsdb_file_txn_from_json(
@@ -110,23 +110,43 @@ ovsdb_file_open(const char *file_name, bool read_only,
  * null pointer.  On failure, returns an ovsdb_error (which the caller must
  * destroy) and sets '*dbp' to NULL. */
 struct ovsdb_error *
+ovsdb_file_open_as_schemas(const char *file_name,
+   const struct shash *schemas,
+   struct ovsdb **dbp)
+{
+return ovsdb_file_open__(file_name, schemas, true, dbp, NULL);
+}
+
+struct ovsdb_error *
 ovsdb_file_open_as_schema(const char *file_name,
   const struct ovsdb_schema *schema,
   struct ovsdb **dbp)
 {
-return ovsdb_file_open__(file_name, schema, true, dbp, NULL);
+struct shash *schemas;
+struct ovsdb_error *err;
+
+schemas = xmalloc(sizeof *schemas);
+shash_init(schemas);
+shash_add(schemas, schema->name, ovsdb_schema_clone(schema));
+
+err = ovsdb_file_open__(file_name, schemas, true, dbp, NULL);
+if (err) {
+ovsdb_schemas_destroy(schemas);
+}
+
+return err;
 }
 
 static struct ovsdb_error *
 ovsdb_file_open_log(const char *file_name, enum ovsdb_log_open_mode open_mode,
-struct ovsdb_log **logp, struct ovsdb_schema **schemap)
+struct ovsdb_log **logp, struct shash **schemasp)
 {
-struct ovsdb_schema *schema = NULL;
+struct shash *schemas = NULL;
 struct ovsdb_log *log = NULL;
 struct ovsdb_error *error;
 struct json *json = NULL;
 
-ovs_assert(logp || schemap);
+ovs_assert(logp || schemasp);
 
 error = ovsdb_log_open(file_name, open_mode, -1, &log);
 if (error) {
@@ -142,8 +162,8 @@ ovsdb_file_open_log(const char *file_name, enum 
ovsdb_log_open_mode open_mode,
 goto error;
 }
 
-if (schemap) {
-error = ovsdb_schema_from_json(json, &schema);
+if (schemasp) {
+error = ovsdb_schemas_from_json(json, &schemas);
 if (error) {
 error = ovsdb_wrap_error(error,
  "failed to parse \"%s\" as ovsdb schema",
@@ -158,8 +178,8 @@ ovsdb_file_open_log(const char *file_name, enum 
ovsdb_log_open_mode open_mode,
 } else {
 ovsdb_log_close(log);
 }
-if (schemap) {
-*schemap = schema;
+if (schemasp) {
+*schemasp = schemas;
 }
 return NULL;
 
@@ -169,21 +189,22 @@ error:
 if (logp) {
 *logp = NULL;
 }
-if (schemap) {
-*schemap = NULL;
+if (schemasp) {
+*schemasp = NULL;
 }
 return error;
 }
 
 static struct ovsdb_error *
 ovsdb_file_open__(const char *file_name,
-  const struct ovsdb_schema *alternate_schema,
+  const struct shash *alternate_schemas,
   bool read_only, struct ovsdb **dbp,
   struct ovsdb_file **filep)
 {
 enum ovsdb_log_open_mode open_mode;
 unsigned int n_transactions;
-struct ovsdb_schema *schema = NULL;
+struct shash *schemas = NULL;
+struct ovsdb_schema *joined;
 struct ovsdb_error *error;
 struct ovsdb_log *log;
 struct json *json;
@@ -194,19 +215,27 @@ ovsdb_file_open__(const char *file_name,
 
 open_mode = read_only ? OVSDB_LOG_READ_ONLY : OVSDB_LOG_READ_WRITE;
 error = ovsdb_file_open_log(file_name, open_mode, &log,
-alternate_schema ? NULL : &schema);
+alternate_schemas ? NULL : &schemas);
 if (error)

[ovs-dev] [join ovsdb schema v2 7/8] ovsdb-tool: add multiple schemas support

2015-06-25 Thread Andy Zhou
Allow ovsdb-tool to accept a schema-list, a list of schema files, in
addition to a single schema file, for all applicable options.

There is no limit on how many schemas can be joined together, but
they can not have a shared schema name and they have to be compatible
for joining. Compatible basically means overlapping tables and columns
have to be of the same type.

A single joined schema, formed by joining schemas from the schema-list,
is used for DB operations.

See man page changes for more details.

Signed-off-by: Andy Zhou 
---
 ovsdb/file.c  |  41 ---
 ovsdb/file.h  |   9 ---
 ovsdb/ovsdb-tool.1.in |  77 +++-
 ovsdb/ovsdb-tool.c| 193 +++---
 ovsdb/ovsdb.c |   6 +-
 ovsdb/ovsdb.h |   7 --
 6 files changed, 198 insertions(+), 135 deletions(-)

diff --git a/ovsdb/file.c b/ovsdb/file.c
index 7709047..74e0c09 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -117,26 +117,6 @@ ovsdb_file_open_as_schemas(const char *file_name,
 return ovsdb_file_open__(file_name, schemas, true, dbp, NULL);
 }
 
-struct ovsdb_error *
-ovsdb_file_open_as_schema(const char *file_name,
-  const struct ovsdb_schema *schema,
-  struct ovsdb **dbp)
-{
-struct shash *schemas;
-struct ovsdb_error *err;
-
-schemas = xmalloc(sizeof *schemas);
-shash_init(schemas);
-shash_add(schemas, schema->name, ovsdb_schema_clone(schema));
-
-err = ovsdb_file_open__(file_name, schemas, true, dbp, NULL);
-if (err) {
-ovsdb_schemas_destroy(schemas);
-}
-
-return err;
-}
-
 static struct ovsdb_error *
 ovsdb_file_open_log(const char *file_name, enum ovsdb_log_open_mode open_mode,
 struct ovsdb_log **logp, struct shash **schemasp)
@@ -527,27 +507,6 @@ ovsdb_file_read_schemas(const char *file_name, struct 
shash **schemasp)
 return ovsdb_file_open_log(file_name, OVSDB_LOG_READ_ONLY, NULL, schemasp);
 }
 
-struct ovsdb_error *
-ovsdb_file_read_schema(const char *file_name, struct ovsdb_schema **schemap)
-{
-struct shash *schemas = NULL;
-struct ovsdb_schema *schema = NULL;
-struct ovsdb_error *err;
-
-ovs_assert(schemap != NULL);
-err = ovsdb_file_read_schemas(file_name, &schemas);
-if (err) {
-goto error;
-}
-
-err = ovsdb_schemas_join(schemas, &schema);
-ovsdb_schemas_destroy(schemas);
-
-error:
-*schemap = schema;
-return err;
-}
-
 
 /* Replica implementation. */
 
diff --git a/ovsdb/file.h b/ovsdb/file.h
index 799c915..f40c2df 100644
--- a/ovsdb/file.h
+++ b/ovsdb/file.h
@@ -29,11 +29,6 @@ struct ovsdb_error *ovsdb_file_open(const char *file_name, 
bool read_only,
 struct ovsdb **, struct ovsdb_file **)
 OVS_WARN_UNUSED_RESULT;
 
-struct ovsdb_error *ovsdb_file_open_as_schema(const char *file_name,
-  const struct ovsdb_schema *,
-  struct ovsdb **)
-OVS_WARN_UNUSED_RESULT;
-
 struct ovsdb_error *ovsdb_file_open_as_schemas(const char *file_name,
const struct shash *,
struct ovsdb **)
@@ -46,10 +41,6 @@ struct ovsdb_error *ovsdb_file_save_copy(const char 
*file_name, int locking,
 
 struct ovsdb_error *ovsdb_file_compact(struct ovsdb_file *);
 
-struct ovsdb_error *ovsdb_file_read_schema(const char *file_name,
-   struct ovsdb_schema **)
-OVS_WARN_UNUSED_RESULT;
-
 struct ovsdb_error *ovsdb_file_read_schemas(const char *file_name,
struct shash **)
 OVS_WARN_UNUSED_RESULT;
diff --git a/ovsdb/ovsdb-tool.1.in b/ovsdb/ovsdb-tool.1.in
index a2f1f22..e6c04c9 100644
--- a/ovsdb/ovsdb-tool.1.in
+++ b/ovsdb/ovsdb-tool.1.in
@@ -12,22 +12,22 @@
 ovsdb\-tool \- Open vSwitch database management utility
 .
 .SH SYNOPSIS
-\fBovsdb\-tool \fR[\fIoptions\fR] \fBcreate \fR[\fIdb\fR [\fIschema\fR]]
+\fBovsdb\-tool \fR[\fIoptions\fR] \fBcreate \fR[\fIdb\fR [\fIschema-list\fR]]
 .br
 \fBovsdb\-tool \fR[\fIoptions\fR] \fBcompact \fR[\fIdb\fR [\fItarget\fR]]
 .br
-\fBovsdb\-tool \fR[\fIoptions\fR] \fBconvert \fR[\fIdb\fR [\fIschema
+\fBovsdb\-tool \fR[\fIoptions\fR] \fBconvert \fR[\fIdb\fR [\fIschema-list
 \fR[\fItarget\fR]]]
 .br
-\fBovsdb\-tool \fR[\fIoptions\fR] \fBneeds\-conversion \fR[\fIdb\fR 
[\fIschema\fR]]
+\fBovsdb\-tool \fR[\fIoptions\fR] \fBneeds\-conversion \fR[\fIdb\fR 
[\fIschema-list\fR]]
 .br
 \fBovsdb\-tool \fR[\fIoptions\fR] \fBdb\-version \fR[\fIdb\fR]
 .br
-\fBovsdb\-tool \fR[\fIoptions\fR] \fBschema\-version \fR[\fIschema\fR]
+\fBovsdb\-tool \fR[\fIoptions\fR] \fBschema\-version \fR[\fIschema-list\fR]
 .br
 \fBovsdb\-tool \fR[\fIoptions\fR] \fBdb\-cksum \fR[\fIdb\fR]
 .br
-\fBovsdb\-tool \fR[\fIoptions\fR] \fBschema\-cksum \fR[\fIschema\fR]
+\fBovsdb\-tool \fR[\fIoptio

[ovs-dev] [join ovsdb schema v2 8/8] tests: add ovsdb-tool join schema tests

2015-06-25 Thread Andy Zhou
Add unit tests for ovsdb-tools that deals with multiple schemas.

Signed-off-by: Andy Zhou 
---
 tests/ovsdb-tool.at | 153 
 1 file changed, 153 insertions(+)

diff --git a/tests/ovsdb-tool.at b/tests/ovsdb-tool.at
index 0d3219b..4a59578 100644
--- a/tests/ovsdb-tool.at
+++ b/tests/ovsdb-tool.at
@@ -341,3 +341,156 @@ AT_CHECK([diff schema schema2], [1], [ignore])
 AT_CHECK([ovsdb-tool needs-conversion db schema2], [0], [yes
 ])
 AT_CLEANUP
+
+m4_divert_push([PREPARE_TESTS])
+[
+
+extended_ordinal_schema () {
+cat <<'EOF'
+{"name": "extended",
+ "tables": {
+   "ordinals": {
+ "columns": {
+   "number": {"type": "integer"},
+   "name": {"type": "string"},
+   "comment": {"type": "string"}},
+ "indexes": [["number"]]}},
+ "version": "5.2.3",
+ "cksum": "22345678 9"}
+EOF
+}
+
+reduced_ordinal_schema () {
+cat <<'EOF'
+{"name": "reduced",
+ "tables": {
+   "ordinals": {
+ "columns": {
+   "number": {"type": "integer"}},
+ "indexes": [["number"]]}},
+ "version": "6.1.3",
+ "cksum": "32345678 9"}
+EOF
+}
+
+incompatible_ordinal_schema () {
+cat <<'EOF'
+{"name": "incompatible",
+ "tables": {
+   "ordinals": {
+ "columns": {
+   "name": {"type": "string"},
+   "number": {"type": "string"}},
+ "indexes": [["number"]]}},
+ "version": "5.1.4",
+ "cksum": "42345678 9"}
+EOF
+}
+
+]
+m4_divert_pop([PREPARE_TESTS])
+
+AT_SETUP([ovsdb-tool create join compatibles])
+AT_KEYWORDS([ovsdb file join positive])
+ordinal_schema > schema
+extended_ordinal_schema > extended
+reduced_ordinal_schema > reduced
+touch .db.~lock~
+AT_CHECK([ovsdb-tool create db 'schema, extended, reduced'], [0], [], [ignore])
+AT_CHECK([[ovsdb-tool transact db '
+["ordinals",
+ {"op": "insert",
+  "table": "ordinals",
+  "row": {"number": 5}}
+]']], [0], [stdout], [ignore])
+AT_CHECK([grep "\"number\":5" db], [0], [ignore])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool reject join incompatible])
+AT_KEYWORDS([ovsdb file join negative])
+ordinal_schema > schema
+incompatible_ordinal_schema > incompatible
+touch .db.~lock~
+AT_CHECK([ovsdb-tool create db 'schema, incompatible'], [0], [], [ignore])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool reject join idential schema])
+AT_KEYWORDS([ovsdb file join negative])
+ordinal_schema > schema0
+ordinal_schema > schema1
+touch .db.~lock~
+AT_CHECK([ovsdb-tool create db 'schema0, schema1'], [1], [], [ignore])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool convert joined schema to single])
+AT_KEYWORDS([ovsdb file join positive])
+ordinal_schema > schema
+extended_ordinal_schema > extended
+touch .db.~lock~
+AT_CHECK([ovsdb-tool create db 'schema, extended'], [0], [], [ignore])
+AT_CHECK([ovsdb-tool convert db schema], [0], [ignore])
+AT_CHECK([ovsdb-tool db-version db], [0], [5.1.3
+])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool convert single schema to joined])
+AT_KEYWORDS([ovsdb file join positive])
+ordinal_schema > schema
+extended_ordinal_schema > extended
+touch .db.~lock~
+AT_CHECK([ovsdb-tool create db 'schema'], [0], [], [ignore])
+AT_CHECK([ovsdb-tool convert db 'schema, extended'], [0], [ignore])
+AT_CHECK([ovsdb-tool db-version db], [0], [ordinals:5.1.3
+extended:5.2.3
+])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool needs-conversion joined schema])
+AT_KEYWORDS([ovsdb file join positive])
+ordinal_schema > schema
+extended_ordinal_schema > extended
+reduced_ordinal_schema > reduced
+touch .db.~lock~
+AT_CHECK([ovsdb-tool create db 'schema, extended'], [0], [], [ignore])
+AT_CHECK([ovsdb-tool needs-conversion db 'schema, reduced'], [0], [yes
+])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool db-version joined schema])
+AT_KEYWORDS([ovsdb file join positive])
+ordinal_schema > schema
+extended_ordinal_schema > extended
+touch .db.~lock~
+AT_CHECK([ovsdb-tool create db 'schema, extended'], [0], [], [ignore])
+AT_CHECK([ovsdb-tool db-version db], [0], [ordinals:5.1.3
+extended:5.2.3
+])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool schema-version joined schema])
+AT_KEYWORDS([ovsdb file join positive])
+ordinal_schema > schema
+extended_ordinal_schema > extended
+AT_CHECK([ovsdb-tool schema-version 'schema, extended'], [0], [ordinals:5.1.3
+extended:5.2.3
+])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool db-cksum joined schema])
+AT_KEYWORDS([ovsdb file join positive])
+ordinal_schema > schema
+extended_ordinal_schema > extended
+AT_CHECK([ovsdb-tool create db 'schema, extended'], [0], [], [ignore])
+AT_CHECK([ovsdb-tool db-cksum db], [0], [ordinals:12345678 9
+extended:22345678 9
+])
+AT_CLEANUP
+
+AT_SETUP([ovsdb-tool schema-cksum joined schema])
+AT_KEYWORDS([ovsdb file join positive])
+ordinal_schema > schema
+extended_ordinal_schema > extended
+AT_CHECK([ovsdb-tool schema-cksum 'schema, extended'], [0], [ordinals:12345678 
9
+extended:22345678 9
+])
+AT_CLEANUP
-- 
1.9.1

___
dev mailing list

[ovs-dev] [join ovsdb schema v2 6/8] ovsdb: add logic for parsing schema file names from a string

2015-06-25 Thread Andy Zhou
Any ovsdb programs that accepts a single schema file name will be
extended to accept multiple file names. This patch implements the
parsing logic for future patches.

Signed-off-by: Andy Zhou 
---
 ovsdb/ovsdb.c | 38 ++
 ovsdb/ovsdb.h |  2 ++
 2 files changed, 40 insertions(+)

diff --git a/ovsdb/ovsdb.c b/ovsdb/ovsdb.c
index 5929306..5305360 100644
--- a/ovsdb/ovsdb.c
+++ b/ovsdb/ovsdb.c
@@ -24,6 +24,7 @@
 #include "ovsdb-parser.h"
 #include "ovsdb-types.h"
 #include "simap.h"
+#include "sset.h"
 #include "table.h"
 #include "transaction.h"
 
@@ -673,3 +674,40 @@ error:
 
 return err;
 }
+
+/*
+ * Parse schema file names from a string.
+ *
+ * A leading comma indicates the default schema name.
+ *
+ * Duplicated filenames will be ignored.   */
+void
+ovsdb_parse_schema_file_names(const char *file_names, struct sset *name_set,
+  const char *default_schema)
+{
+const char *delimiter=", \t\r\n";
+const char *filename;
+char *saveptr;
+char *fns;
+
+if (!file_names) {
+sset_add(name_set, default_schema);
+return;
+}
+
+if (*file_names == ',') {
+sset_add(name_set, default_schema);
+}
+
+fns = xstrdup(file_names);
+filename = strtok_r(fns, delimiter, &saveptr);
+if (!filename) {
+filename = default_schema;
+}
+
+while(filename) {
+sset_add(name_set, filename);
+filename = strtok_r(NULL, delimiter, &saveptr);
+}
+free(fns);
+}
diff --git a/ovsdb/ovsdb.h b/ovsdb/ovsdb.h
index 2e7f7e0..b61b6ba 100644
--- a/ovsdb/ovsdb.h
+++ b/ovsdb/ovsdb.h
@@ -56,6 +56,8 @@ struct shash *ovsdb_schemas_clone(const struct shash 
*schemas);
 struct ovsdb_error *ovsdb_schemas_from_files(const struct sset *files,
  struct shash** )
 OVS_WARN_UNUSED_RESULT;
+void ovsdb_parse_schema_file_names(const char *file_names, struct sset *names,
+   const char *default_schema);
 void ovsdb_schemas_destroy(struct shash *schemas);
 struct json *ovsdb_schemas_to_json(const struct shash *schemas)
 OVS_WARN_UNUSED_RESULT;
-- 
1.9.1

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


Re: [ovs-dev] [PATCH]: bfd: display last wall clock time of last flap

2015-06-25 Thread Sabyasachi Sengupta



It's much easier for a script to parse a number than a date.


Well, I tried to use time_msec instead of time_wall_msec() in an attempt to 
see how the output would look like.


To me, the original proposal of printing wall clock time

--
[root@rtr-29-225-196-232 ~]# ovs-appctl bfd/show
 port3.4094 
 Forwarding: true
 Detect Multiplier: 3
 Concatenated Path Down: false
 TX Interval: Approx 500ms
 RX Interval: Approx 500ms
 Detect Time: now -1116ms
 Next TX Time: now -436ms
 Last TX Time: now +19ms
 Last Flap Time: 2015-06-25 00:36:22.372 <<<
[..] 

made lot more sense than printing something like

[sabyasse@sabyasg-lnx-90 sabyasse]$ sudo ovs-appctl bfd/show
 enp0s10.4094 
Forwarding: true
Detect Multiplier: 3
Concatenated Path Down: false
TX Interval: Approx 2000ms
RX Interval: Approx 2000ms
Detect Time: now -4944ms
Next TX Time: now -728ms
Last TX Time: now +1012ms
Last Flap Time: now +2617332ms 
[..]

Again, the whole purpose for this was to give some kind of heads up to the 
user about flap and when that happened. (FWIW, I was also thinking of 
printing flap_count in bfd/show as well if that made sense).



So if there is agreement in printing wall clock time, I'm fine with 
printing GMT as suggested in this thread. Otherwise, if following existing 
convention for displaying msecs only in bfd/show is a concern, I'm also fine 
in NOT displaying "Last Flap Time" at all from ovs-appctl bfd/show, but just 
saving the wall clock time in ovsdb as scripts can still read it from 
there.

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


Re: [ovs-dev] [PATCH] tunneling: Don't match on source IP address for native tunnels.

2015-06-25 Thread Jesse Gross
On Wed, Jun 24, 2015 at 8:40 PM, Pravin Shelar  wrote:
> On Wed, Jun 24, 2015 at 2:55 PM, Jesse Gross  wrote:
>> When doing native tunneling, we look at packets destined to the
>> local port to see if they match tunnel protocols that we should
>> intercept. The criteria are IP protocol, destination UDP port, etc.
>>
>> However, we also look at the source IP address of the packets. This
>> should be a function of the port-based tunnel layer and not the
>> tunnel receive code itself. For comparison, the kernel tunnel code
>> has no idea about the IP addresses of its link partners. If port
>> based tunnel is desired, it can be handled using the normal port
>> tunnel layer, regardless of whether the packets originally came
>> from userspace or the kernel.
>>
>> For port based tunneling, this bug has no effect - the check is
>> simply redundant. However, it breaks flow-based native tunnels
>> because the remote IP address is not known at port creation time.
>>
>> CC: Pravin Shelar 
>> Reported-by: David Griswold 
>> Signed-off-by: Jesse Gross 
>
> Can you add this test case?
[...]
> Patch looks good to me.

Thanks, I added a test case and pushed to branch-2.4 and master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ofp-actions: Support mixing "conjunction" and "note" actions.

2015-06-25 Thread Andy Zhou
On Mon, Jun 15, 2015 at 1:58 PM, Ben Pfaff  wrote:
> It doesn't make sense to mix "conjunction" actions with most other kinds
> of actions.  That's because flows with "conjunction" actions aren't ever
> actually executed, so any actions mixed up with them would never do
> anything useful.  "note" actions are a little different because they never
> do anything useful anyway: they are just there to allow a controller to
> annotate flows.  It makes as much sense to annotate a flow with
> "conjunction" actions as it does to annotate any other flow, so this
> commit makes this possible.
>
> Requested-by: Soner Sevinc 
> Signed-off-by: Ben Pfaff 
> -
>  static void
>  get_conjunctions(const struct ofputil_flow_mod *fm,
>   struct cls_conjunction **conjsp, size_t *n_conjsp)
> @@ -4420,23 +4414,31 @@ get_conjunctions(const struct ofputil_flow_mod *fm,
>  struct cls_conjunction *conjs = NULL;
>  int n_conjs = 0;
>
> -if (is_conjunction(fm->ofpacts, fm->ofpacts_len)) {
> -const struct ofpact *ofpact;
> -int i;
> +n_conjs = 0;
I don't think this line is necessary. Otherwise, looks good.

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


Re: [ovs-dev] [PATCH] expr: Fix typo in comment.

2015-06-25 Thread Ben Pfaff
Thanks, applied.

On Thu, Jun 25, 2015 at 01:40:05PM -0700, Andy Zhou wrote:
> Acked-by: Andy Zhou 
> 
> On Thu, Jun 25, 2015 at 11:27 AM, Ben Pfaff  wrote:
> > Signed-off-by: Ben Pfaff 
> > ---
> >  ovn/lib/expr.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ovn/lib/expr.h b/ovn/lib/expr.h
> > index 501152e..b411e65 100644
> > --- a/ovn/lib/expr.h
> > +++ b/ovn/lib/expr.h
> > @@ -293,7 +293,7 @@ bool expr_relop_from_token(enum lex_type type, enum 
> > expr_relop *relop);
> >   *   have grandchildren of its own type.
> >   *
> >   *   As a consequence, every nonterminal node at the same distance 
> > from the
> > - *   root of the root has the same type.
> > + *   root has the same type.
> >   *
> >   * - EXPR_T_AND and EXPR_T_OR nodes must have at least two children.
> >   *
> > --
> > 2.1.3
> >
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] ovn: Take advantage of OVSDB garbage collection in OVN_Northbound schema.

2015-06-25 Thread Ben Pfaff
Until now, the OVN_Northbound schema has been designed to sidestep a
weakness in the OVSDB protocol when a column has a great deal of data in
it.  In the current OVSDB protocol, whenever a column changes, the entire
new value of the column is sent to all of the clients that are monitoring
that column.  That means that adding or removing a small amount of data,
say 1 element in a set, requires sending all of the data, which is
expensive if the column has a lot of data.

One example of a column with potential to have a lot of data is the set of
ports within a logical switch, if a logical switch has a large number of
ports.  Thus, the existing OVN_Northbound schema has each Logical_Port
point to its containing Logical_Switch instead of the other way around.
This sidesteps the problem because it does not use any large columns.

The tradeoff that this forces, however, is that the schema cannot take
advantage of OVSDB's garbage collection feature, where it automatically
deletes rows that are unreferenced.  That's a problem for Neutron because
of Neutron-internal races between deletion of a Logical_Switch and
creation of new Logical_Ports on the switch being deleted.  When such a
race happens, OVSDB refuses to delete the Logical_Switch because of
references to it from the newly created Logical_Port (although Neutron
does delete the pre-existing logical ports).

To solve the problem, this commit changes the OVN_Northbound schema to
use a set of ports within Logical_Switch.  That will lead to large columns
for large logical switches; I plan to address that (though I don't have
code written) by enhancing the OVSDB protocol.  With this commit applied,
the database will automatically cascade deleting a logical switch row to
delete all of its ports, ACLs, and its router port (if any).

This commit makes some pretty pervasive changes to ovn-northd, but they
are mostly beneficial to the code readability because now it becomes
possible to trivially iterate through the ports that belong to a switch,
which was difficult before the schema change.

This commit will break the Neutron integration until that is changed to
handle the new database schema.

CC: Aaron Rosen 
Signed-off-by: Ben Pfaff 
---
 ovn/northd/ovn-northd.c | 318 ++--
 ovn/ovn-nb.ovsschema|  39 +++---
 ovn/ovn-nb.xml  |  74 ++-
 ovn/ovn-nbctl.c | 116 ++
 4 files changed, 282 insertions(+), 265 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index f37df77..c790a90 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -281,139 +281,116 @@ build_pipeline(struct northd_context *ctx)
 }
 
 /* Table 0: Ingress port security. */
-const struct nbrec_logical_port *lport;
-NBREC_LOGICAL_PORT_FOR_EACH (lport, ctx->ovnnb_idl) {
-struct ds match = DS_EMPTY_INITIALIZER;
-ds_put_cstr(&match, "inport == ");
-json_string_escape(lport->name, &match);
-build_port_security("eth.src",
-lport->port_security, lport->n_port_security,
-&match);
-pipeline_add(&pc, lport->lswitch, 0, 50, ds_cstr(&match),
- lport_is_enabled(lport) ? "next;" : "drop;");
-ds_destroy(&match);
-}
-
-/* Table 1: Destination lookup, broadcast and multicast handling (priority
- * 100). */
 NBREC_LOGICAL_SWITCH_FOR_EACH (lswitch, ctx->ovnnb_idl) {
-struct ds actions;
-
-ds_init(&actions);
-NBREC_LOGICAL_PORT_FOR_EACH (lport, ctx->ovnnb_idl) {
-if (lport->lswitch == lswitch && lport_is_enabled(lport)) {
-ds_put_cstr(&actions, "outport = ");
-json_string_escape(lport->name, &actions);
-ds_put_cstr(&actions, "; next; ");
-}
+for (size_t i = 0; i < lswitch->n_ports; i++) {
+const struct nbrec_logical_port *lport = lswitch->ports[i];
+struct ds match = DS_EMPTY_INITIALIZER;
+ds_put_cstr(&match, "inport == ");
+json_string_escape(lport->name, &match);
+build_port_security("eth.src",
+lport->port_security, lport->n_port_security,
+&match);
+pipeline_add(&pc, lswitch, 0, 50, ds_cstr(&match),
+ lport_is_enabled(lport) ? "next;" : "drop;");
+ds_destroy(&match);
 }
-ds_chomp(&actions, ' ');
-
-pipeline_add(&pc, lswitch, 1, 100, "eth.dst[40]", ds_cstr(&actions));
-ds_destroy(&actions);
 }
 
-/* Table 1: Destination lookup, unicast handling (priority 50),  */
-struct unknown_actions {
-struct hmap_node hmap_node;
-const struct nbrec_logical_switch *ls;
-struct ds actions;
-};
-struct hmap unknown_actions = HMAP_INITIALIZER(&unknown_actions);
-NBREC_LOGICAL_PORT_FOR_EACH (lport, ctx->ovnnb_idl)

Re: [ovs-dev] [PATCH] ovn: Take advantage of OVSDB garbage collection in OVN_Northbound schema.

2015-06-25 Thread Aaron Rosen
Awesome thanks Ben! I'll update neutron tomorrow morning to work with this.


> On Jun 25, 2015, at 5:35 PM, Ben Pfaff  wrote:
> 
> Until now, the OVN_Northbound schema has been designed to sidestep a
> weakness in the OVSDB protocol when a column has a great deal of data in
> it.  In the current OVSDB protocol, whenever a column changes, the entire
> new value of the column is sent to all of the clients that are monitoring
> that column.  That means that adding or removing a small amount of data,
> say 1 element in a set, requires sending all of the data, which is
> expensive if the column has a lot of data.
> 
> One example of a column with potential to have a lot of data is the set of
> ports within a logical switch, if a logical switch has a large number of
> ports.  Thus, the existing OVN_Northbound schema has each Logical_Port
> point to its containing Logical_Switch instead of the other way around.
> This sidesteps the problem because it does not use any large columns.
> 
> The tradeoff that this forces, however, is that the schema cannot take
> advantage of OVSDB's garbage collection feature, where it automatically
> deletes rows that are unreferenced.  That's a problem for Neutron because
> of Neutron-internal races between deletion of a Logical_Switch and
> creation of new Logical_Ports on the switch being deleted.  When such a
> race happens, OVSDB refuses to delete the Logical_Switch because of
> references to it from the newly created Logical_Port (although Neutron
> does delete the pre-existing logical ports).
> 
> To solve the problem, this commit changes the OVN_Northbound schema to
> use a set of ports within Logical_Switch.  That will lead to large columns
> for large logical switches; I plan to address that (though I don't have
> code written) by enhancing the OVSDB protocol.  With this commit applied,
> the database will automatically cascade deleting a logical switch row to
> delete all of its ports, ACLs, and its router port (if any).
> 
> This commit makes some pretty pervasive changes to ovn-northd, but they
> are mostly beneficial to the code readability because now it becomes
> possible to trivially iterate through the ports that belong to a switch,
> which was difficult before the schema change.
> 
> This commit will break the Neutron integration until that is changed to
> handle the new database schema.
> 
> CC: Aaron Rosen 
> Signed-off-by: Ben Pfaff 
> ---
> ovn/northd/ovn-northd.c | 318 ++--
> ovn/ovn-nb.ovsschema|  39 +++---
> ovn/ovn-nb.xml  |  74 ++-
> ovn/ovn-nbctl.c | 116 ++
> 4 files changed, 282 insertions(+), 265 deletions(-)
> 
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index f37df77..c790a90 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -281,139 +281,116 @@ build_pipeline(struct northd_context *ctx)
> }
> 
> /* Table 0: Ingress port security. */
> -const struct nbrec_logical_port *lport;
> -NBREC_LOGICAL_PORT_FOR_EACH (lport, ctx->ovnnb_idl) {
> -struct ds match = DS_EMPTY_INITIALIZER;
> -ds_put_cstr(&match, "inport == ");
> -json_string_escape(lport->name, &match);
> -build_port_security("eth.src",
> -lport->port_security, lport->n_port_security,
> -&match);
> -pipeline_add(&pc, lport->lswitch, 0, 50, ds_cstr(&match),
> - lport_is_enabled(lport) ? "next;" : "drop;");
> -ds_destroy(&match);
> -}
> -
> -/* Table 1: Destination lookup, broadcast and multicast handling 
> (priority
> - * 100). */
> NBREC_LOGICAL_SWITCH_FOR_EACH (lswitch, ctx->ovnnb_idl) {
> -struct ds actions;
> -
> -ds_init(&actions);
> -NBREC_LOGICAL_PORT_FOR_EACH (lport, ctx->ovnnb_idl) {
> -if (lport->lswitch == lswitch && lport_is_enabled(lport)) {
> -ds_put_cstr(&actions, "outport = ");
> -json_string_escape(lport->name, &actions);
> -ds_put_cstr(&actions, "; next; ");
> -}
> +for (size_t i = 0; i < lswitch->n_ports; i++) {
> +const struct nbrec_logical_port *lport = lswitch->ports[i];
> +struct ds match = DS_EMPTY_INITIALIZER;
> +ds_put_cstr(&match, "inport == ");
> +json_string_escape(lport->name, &match);
> +build_port_security("eth.src",
> +lport->port_security, lport->n_port_security,
> +&match);
> +pipeline_add(&pc, lswitch, 0, 50, ds_cstr(&match),
> + lport_is_enabled(lport) ? "next;" : "drop;");
> +ds_destroy(&match);
> }
> -ds_chomp(&actions, ' ');
> -
> -pipeline_add(&pc, lswitch, 1, 100, "eth.dst[40]", ds_cstr(&actions));
> -ds_destroy(&actions);
> }
> 
> -/* Table 1: Destination loo

Re: [ovs-dev] [PATCH] ovn: Take advantage of OVSDB garbage collection in OVN_Northbound schema.

2015-06-25 Thread Ben Pfaff
Thanks.  Of course this patch needs review on the OVS side as well; that
might take a few days.

On Thu, Jun 25, 2015 at 11:51:57PM +, Aaron Rosen wrote:
> Awesome thanks Ben! I'll update neutron tomorrow morning to work with this.
> 
> 
> > On Jun 25, 2015, at 5:35 PM, Ben Pfaff  wrote:
> > 
> > Until now, the OVN_Northbound schema has been designed to sidestep a
> > weakness in the OVSDB protocol when a column has a great deal of data in
> > it.  In the current OVSDB protocol, whenever a column changes, the entire
> > new value of the column is sent to all of the clients that are monitoring
> > that column.  That means that adding or removing a small amount of data,
> > say 1 element in a set, requires sending all of the data, which is
> > expensive if the column has a lot of data.
> > 
> > One example of a column with potential to have a lot of data is the set of
> > ports within a logical switch, if a logical switch has a large number of
> > ports.  Thus, the existing OVN_Northbound schema has each Logical_Port
> > point to its containing Logical_Switch instead of the other way around.
> > This sidesteps the problem because it does not use any large columns.
> > 
> > The tradeoff that this forces, however, is that the schema cannot take
> > advantage of OVSDB's garbage collection feature, where it automatically
> > deletes rows that are unreferenced.  That's a problem for Neutron because
> > of Neutron-internal races between deletion of a Logical_Switch and
> > creation of new Logical_Ports on the switch being deleted.  When such a
> > race happens, OVSDB refuses to delete the Logical_Switch because of
> > references to it from the newly created Logical_Port (although Neutron
> > does delete the pre-existing logical ports).
> > 
> > To solve the problem, this commit changes the OVN_Northbound schema to
> > use a set of ports within Logical_Switch.  That will lead to large columns
> > for large logical switches; I plan to address that (though I don't have
> > code written) by enhancing the OVSDB protocol.  With this commit applied,
> > the database will automatically cascade deleting a logical switch row to
> > delete all of its ports, ACLs, and its router port (if any).
> > 
> > This commit makes some pretty pervasive changes to ovn-northd, but they
> > are mostly beneficial to the code readability because now it becomes
> > possible to trivially iterate through the ports that belong to a switch,
> > which was difficult before the schema change.
> > 
> > This commit will break the Neutron integration until that is changed to
> > handle the new database schema.
> > 
> > CC: Aaron Rosen 
> > Signed-off-by: Ben Pfaff 
> > ---
> > ovn/northd/ovn-northd.c | 318 
> > ++--
> > ovn/ovn-nb.ovsschema|  39 +++---
> > ovn/ovn-nb.xml  |  74 ++-
> > ovn/ovn-nbctl.c | 116 ++
> > 4 files changed, 282 insertions(+), 265 deletions(-)
> > 
> > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> > index f37df77..c790a90 100644
> > --- a/ovn/northd/ovn-northd.c
> > +++ b/ovn/northd/ovn-northd.c
> > @@ -281,139 +281,116 @@ build_pipeline(struct northd_context *ctx)
> > }
> > 
> > /* Table 0: Ingress port security. */
> > -const struct nbrec_logical_port *lport;
> > -NBREC_LOGICAL_PORT_FOR_EACH (lport, ctx->ovnnb_idl) {
> > -struct ds match = DS_EMPTY_INITIALIZER;
> > -ds_put_cstr(&match, "inport == ");
> > -json_string_escape(lport->name, &match);
> > -build_port_security("eth.src",
> > -lport->port_security, lport->n_port_security,
> > -&match);
> > -pipeline_add(&pc, lport->lswitch, 0, 50, ds_cstr(&match),
> > - lport_is_enabled(lport) ? "next;" : "drop;");
> > -ds_destroy(&match);
> > -}
> > -
> > -/* Table 1: Destination lookup, broadcast and multicast handling 
> > (priority
> > - * 100). */
> > NBREC_LOGICAL_SWITCH_FOR_EACH (lswitch, ctx->ovnnb_idl) {
> > -struct ds actions;
> > -
> > -ds_init(&actions);
> > -NBREC_LOGICAL_PORT_FOR_EACH (lport, ctx->ovnnb_idl) {
> > -if (lport->lswitch == lswitch && lport_is_enabled(lport)) {
> > -ds_put_cstr(&actions, "outport = ");
> > -json_string_escape(lport->name, &actions);
> > -ds_put_cstr(&actions, "; next; ");
> > -}
> > +for (size_t i = 0; i < lswitch->n_ports; i++) {
> > +const struct nbrec_logical_port *lport = lswitch->ports[i];
> > +struct ds match = DS_EMPTY_INITIALIZER;
> > +ds_put_cstr(&match, "inport == ");
> > +json_string_escape(lport->name, &match);
> > +build_port_security("eth.src",
> > +lport->port_security, 
> > lport->n_port_security,
> > +&match);
> > +pipeline_add(