[ovs-dev] [PATCH] lib/route-table-*: Fix non-Linux builds

2014-11-25 Thread YAMAMOTO Takashi
Fix build failures introduced by
commit 1bc50ef389d40be9ee215e66b49cf66fcbdb9eeb
("dpctl: Fix crash.")

Signed-off-by: YAMAMOTO Takashi 
---
 lib/route-table-bsd.c  | 2 +-
 lib/route-table-stub.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/route-table-bsd.c b/lib/route-table-bsd.c
index 9ebfaa3..9793931 100644
--- a/lib/route-table-bsd.c
+++ b/lib/route-table-bsd.c
@@ -143,6 +143,6 @@ route_table_wait(void)
 }
 
 void
-ovs_router_unixctl_register(void)
+ovs_router_init(void)
 {
 }
diff --git a/lib/route-table-stub.c b/lib/route-table-stub.c
index b7686e4..f91c6c7 100644
--- a/lib/route-table-stub.c
+++ b/lib/route-table-stub.c
@@ -27,7 +27,7 @@ ovs_router_lookup(ovs_be32 ip_dst OVS_UNUSED, char 
output_bridge[] OVS_UNUSED,
 }
 
 void
-ovs_router_unixctl_register(void)
+ovs_router_init(void)
 {
 }
 
-- 
1.9.4

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


[ovs-dev] [PATCH] ovs-ofctl.at: Fix test for TCP flags

2014-11-25 Thread YAMAMOTO Takashi
Update the test after commit 847ddeab372220170f431f1b9dd504237066729a
("meta-flow: Add support for ONF OpenFlow 1.3 extension ONFOXM_ET_TCP_FLAGS.")

Signed-off-by: YAMAMOTO Takashi 
---
 tests/ovs-ofctl.at | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index eafaa63..d616656 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2340,15 +2340,21 @@ AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-nxm], [0],
   [NXM_OF_ETH_TYPE(0800), NXM_OF_IP_PROTO(06), NXM_NX_TCP_FLAGS(0fff)
 ])
 
-# Check that marshaling in OXM for OF1.2 through OF1.4 gives OXM
-# headers except for TCP flags, which didn't have an OXM definition until
-# OF1.5.
-for version in OpenFlow12 OpenFlow13 OpenFlow14; do
+# Check that marshaling in OXM for OF1.2 gives OXM headers except for
+# TCP flags, which didn't have an OXM definition.
+for version in OpenFlow12; do
 AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm $version], [0],
   [OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), NXM_NX_TCP_FLAGS(0fff)
 ])
 done
 
+# OpenFlow 1.3 and later has ONF extension for TCP flags.
+for version in OpenFlow13 OpenFlow14; do
+AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm $version], [0],
+  [OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), ONFOXM_ET_TCP_FLAGS(0fff)
+])
+done
+
 # OpenFlow 1.5 added an OXM header for TCP flags:
 AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow15], [0],
   [OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), OXM_OF_TCP_FLAGS(0fff)
-- 
1.9.4

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


[ovs-dev] Bug#768095: openvswitch-datapath-dkms fails to build on Debian 7.7 3.2.0-4-amd64 (3.2.63-2+deb7u1)

2014-11-25 Thread Thomas Goirand
On 11/25/2014 12:32 AM, Jonathan Dupart wrote:
> Hi,
> 
> I am looking for a sponsor for the package openvswitch to correct bug
> #768095.
> 
> This bug prevents building the openvswitch kernel module with the last
> stable kernel.
> 
> As openvswitch maintainer requested an NMU, i built a package [1] with
> the patch already used by Ubuntu to fix the same bug [2] (i use this fix
> since 10 days on live servers).
> 
> I attached a full debdiff of the changes.
> 
>   [1]: http://mentors.debian.net/package/openvswitch
>
> http://mentors.debian.net/debian/pool/main/o/openvswitch/openvswitch_1.4.2+git20120612-9.1~deb7u1.1.dsc
>   [2]: https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/1379201
> 
> 
> Regards,

Hi Jonathan,

Before uploading such an update, this has to be discussed with the
release team, as part of the stable-proposed-updates procedure. Please
open a bug against the release.debian.org pseudo-package, and get
approval by the release team (when writing such bug report, you must
send a debdiff between the version in Wheezy and the proposed version).

When the release team has approved your change, then I can sponsor it.

Cheers,

Thomas Goirand (zigo)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] lib/route-table-*: Fix non-Linux builds

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 05:01:13PM +0900, YAMAMOTO Takashi wrote:
> Fix build failures introduced by
> commit 1bc50ef389d40be9ee215e66b49cf66fcbdb9eeb
> ("dpctl: Fix crash.")
> 
> Signed-off-by: YAMAMOTO Takashi 

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


Re: [ovs-dev] [PATCH] datapath/linux: add KCFLAGS var to modules Makefile.main.in

2014-11-25 Thread Ben Pfaff
On Tue, Oct 28, 2014 at 07:51:16AM -0700, Ben Pfaff wrote:
> From: Alexandru Ardelean 
> 
> This is mostly required because of GCC 4.9 which seems
> to error out with:
>   openvswitch/datapath/linux/datapath.c:2108:10:
>error: macro "DATE" might prevent reproducible builds
> 
> We would have wanted to add '-Wno-error=date-time' directly
> but that would be too specific, so we decided to add
> a generic make flag and configure it with what we need.
> 
> Signed-off-by: Alexandru Ardelean 
> ---
> This is from https://github.com/openvswitch/ovs/pull/18.  I'm posting it
> here for review because I'm not the right person to review it and I'm not
> sure that the OVS kernel maintainers are subscribed to the github issues
> tracker.

Pravin or Jesse, would you mind take a look at this?

Thanks,

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


[ovs-dev] [PATCH] datapath-windows: Broken build on MSVC

2014-11-25 Thread Alin Serdean
Removing unused local variable.


Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Flow.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index ceac37f..1257377 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -411,8 +411,6 @@ _FlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
  UINT32 *replyLen)
 {
 NTSTATUS rc = STATUS_SUCCESS;
-POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE)
-  (usrParamsCtx->ovsInstance);
 POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
 PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg);
 POVS_HDR ovsHdr = &(msgIn->ovsHdr);
-- 
1.9.4.msysgit.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath/linux: add KCFLAGS var to modules Makefile.main.in

2014-11-25 Thread Thomas Graf
On 11/25/14 at 07:47am, Ben Pfaff wrote:
> On Tue, Oct 28, 2014 at 07:51:16AM -0700, Ben Pfaff wrote:
> > From: Alexandru Ardelean 
> > 
> > This is mostly required because of GCC 4.9 which seems
> > to error out with:
> >   openvswitch/datapath/linux/datapath.c:2108:10:
> >error: macro "DATE" might prevent reproducible builds
> > 
> > We would have wanted to add '-Wno-error=date-time' directly
> > but that would be too specific, so we decided to add
> > a generic make flag and configure it with what we need.
> > 
> > Signed-off-by: Alexandru Ardelean 
> > ---
> > This is from https://github.com/openvswitch/ovs/pull/18.  I'm posting it
> > here for review because I'm not the right person to review it and I'm not
> > sure that the OVS kernel maintainers are subscribed to the github issues
> > tracker.
> 
> Pravin or Jesse, would you mind take a look at this?

Alexandru,

You can already pass in additional build flags through the Kbuild
EXTRA_CFLAGS:

$ ./configure --with-linux=...
$ cd datapath && make EXTRA_CFLAGS=-Wno-error=date-time

Does that suit your needs?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] ofport_open and alloc_ofp_port

2014-11-25 Thread Ben Pfaff
On Thu, Nov 20, 2014 at 12:38:47PM -0800, Prashanth Mundkur wrote:
> I'm able to resolve the assert with the following patch to
> ofproto.c:ofport_open():
> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 653b2a8..48ceff7 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -2177,6 +2177,8 @@ ofport_open(struct ofproto *ofproto,
>  ofproto_port->ofp_port = alloc_ofp_port(ofproto,
>  ofproto_port->name);
>  }
> +} else {
> +ofport_set_usage(ofproto, ofproto_port->ofp_port, LLONG_MAX);
>  }
>  pp->port_no = ofproto_port->ofp_port;
>  netdev_get_etheraddr(netdev, pp->hw_addr);
> 
> 
> The port is created in the following sequence in ofproto.c:update_port():
> 
> netdev = (!ofproto_port_query_by_name(ofproto, name, &ofproto_port)
>   ? ofport_open(ofproto, &ofproto_port, &pp)
>   : NULL);
> 
> My ofproto implementation of port_query_by_name returns a ofport of 1, which
> causes ofport_open to skip the alloc_ofp_port() to update the usage hmap.  
> When
> I do the update by adding the line in the above patch, the assert goes away.
> 
> Is this a bug, or am I not using the api correctly?

The ofproto layer (not the implementation of it in e.g. ofproto-dpif)
expects to assign OpenFlow port numbers.  It sounds like your
implementation is trying to assign OpenFlow port numbers.  That may be
the problem.  That said, since ofproto-dpif is the main implementation
of the ofproto layer, there could easily be bugs in the layering.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] vswitch.xml: Describe NetFlow passive timeout.

2014-11-25 Thread Ben Pfaff
On Wed, Nov 05, 2014 at 08:35:56AM -0800, Ben Pfaff wrote:
> Reported-by: Martin Vizvary 
> Signed-off-by: Ben Pfaff 

I'd appreciate a review.

Thanks,

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


Re: [ovs-dev] [PATCH] Makefile: Add static check for misuse of LITTLE_ENDIAN and BIG_ENDIAN.

2014-11-25 Thread Ben Pfaff
On Tue, Nov 04, 2014 at 11:42:55AM -0800, Ben Pfaff wrote:
> I've pointed out two misuses of these macros in review in the last week,
> by different authors.  It's time to make it difficult to screw this up.
> 
> Signed-off-by: Ben Pfaff 

I'd appreciate a review.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-vsctl: Prevent creating duplicate VLAN bridges.

2014-11-25 Thread Ben Pfaff
On Thu, Nov 06, 2014 at 02:59:48PM -0800, Ben Pfaff wrote:
> ovs-vsctl has the concept of a VLAN (or "fake") bridge, which is a
> sort of a sub-bridge that receives only packets on a particular VLAN.
> There is no way to distinguish two VLAN bridges with the same parent on the
> same VLAN, but until now ovs-vsctl did not prevent creating duplicates or
> report them.  This commit fixes the problem.
> 
> Reported-by: rwxybh
> Reported-at: https://github.com/openvswitch/ovs/issues/21
> Signed-off-by: Ben Pfaff 

I'd appreciate a review.

Thanks,

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


Re: [ovs-dev] [PATCH] openflow: Note that element 4 of enum ofp15_group_mod_command is reserved

2014-11-25 Thread Ben Pfaff
On Fri, Nov 21, 2014 at 10:15:32AM +0900, Simon Horman wrote:
> The spec has been clarified to note that element 4 of
> enum ofp_group_mod_command is reserved. This patch reflects
> that change.
> 
> ONF-JIRA: EXT-350
> Signed-off-by: Simon Horman 

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


Re: [ovs-dev] [PATCH] openflow: Use *_array_len names in struct ofp15_bucket and ofp15_group_mod

2014-11-25 Thread Ben Pfaff
On Fri, Nov 21, 2014 at 10:16:23AM +0900, Simon Horman wrote:
> The spec has been clarified to use _list_len in palce of _list_len
> terminology to make it clearer that the data is not an ordered list
> (it is a set). The code present in Open vSwitch already avoided
> the _list_len terminology. This change brings the code into
> line with the updated spec.
> 
> ONF-JIRA: EXT-350
> Signed-off-by: Simon Horman 

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


Re: [ovs-dev] [PATCH] Makefile: Add static check for misuse of LITTLE_ENDIAN and BIG_ENDIAN.

2014-11-25 Thread Thomas Graf
On 11/04/14 at 11:42am, Ben Pfaff wrote:
> I've pointed out two misuses of these macros in review in the last week,
> by different authors.  It's time to make it difficult to screw this up.
> 
> Signed-off-by: Ben Pfaff 
> ---
>  Makefile.am |   17 +
>  1 file changed, 17 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 343dffa..caf9408 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -255,6 +255,23 @@ check-assert-h-usage:
>fi
>  .PHONY: check-assert-h-usage
>  
> +# Check that LITTLE_ENDIAN and BIG_ENDIAN are not used unless BYTE_ORDER is
> +# also mentioned.  ( always defines the former two constants.  They
> +# must be compared to BYTE_ORDER to get the machine's correct endianness.  
> But
> +# it is better to use WORDS_BIGENDIAN.)
> +ALL_LOCAL += check-endian
> +check-endian:
> + @if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1 && \
> + (cd $(srcdir) && git --no-pager grep -l -E \
> + -e 'BIG_ENDIAN|LITTLE_ENDIAN' --and --not -e 'BYTE_ORDER' | \
> + $(EGREP) -v '^datapath/'); \
> + then \
> + echo "See above for list of files that misuse LITTLE""_ENDIAN"; \
> + echo "or BIG""_ENDIAN.  Please use WORDS_BIGENDIAN instead."; \
> + exit 1; \
> +  fi
> +.PHONY: check-assert-h-usage

Missed s// above ^^^, should be check-endian
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] Makefile: Add static check for misuse of LITTLE_ENDIAN and BIG_ENDIAN.

2014-11-25 Thread Thomas Graf
On 11/25/14 at 05:19pm, Thomas Graf wrote:
> On 11/04/14 at 11:42am, Ben Pfaff wrote:
> > I've pointed out two misuses of these macros in review in the last week,
> > by different authors.  It's time to make it difficult to screw this up.
> > 
> > Signed-off-by: Ben Pfaff 
> 
> Missed s// above ^^^, should be check-endian

Otherwise:

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


Re: [ovs-dev] [PATCH] ovs-ofctl: Implementation of eviction on the basis of Importance

2014-11-25 Thread Ben Pfaff
On Fri, Nov 21, 2014 at 03:13:40PM +0530, Saloni Jain wrote:
> >There is no need for this code in handle_table_desc_request() because
> >higher-level code has already checked these properties for
> >correctness:
> >    ofpbuf_use_const(&msg, request, ntohs(request->length));
> >   ofpraw_pull_assert(&msg);
> >   if (ofpbuf_size(&msg) || ofpmp_more(request)) {
> >       return OFPERR_OFPTFFC_EPERM;
> >    }
> 
> We have rechecked the code, the higher-level code is not performing
> these checks.  Also same checking has been done for
> handle_table_feature_request()

ofptype_decode() and related functions check that the length of a
message agrees with the specification given in enum ofpraw in
ofp-msgs.h, so there is no need to do that again here.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/2] ofproto-dpif: fix OF1.4 packet-in wire protocol version

2014-11-25 Thread Ben Pfaff
On Fri, Nov 21, 2014 at 11:44:24AM -0800, Shu Shen wrote:
> Previously even when OF1.4 is used between the controller and the
> switch, packet-in messages has OF1.3 as wire protocol version.
> 
> Signed-off-by: Shu Shen 

It looks like this was already fixed by Simon Horman's patch last week.

I'll take a look at patch 1 now and see whether the test passes.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-vsctl: Prevent creating duplicate VLAN bridges.

2014-11-25 Thread Thomas Graf
On 11/06/14 at 02:59pm, Ben Pfaff wrote:
> ovs-vsctl has the concept of a VLAN (or "fake") bridge, which is a
> sort of a sub-bridge that receives only packets on a particular VLAN.
> There is no way to distinguish two VLAN bridges with the same parent on the
> same VLAN, but until now ovs-vsctl did not prevent creating duplicates or
> report them.  This commit fixes the problem.
> 
> Reported-by: rwxybh
> Reported-at: https://github.com/openvswitch/ovs/issues/21
> Signed-off-by: Ben Pfaff 

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


Re: [ovs-dev] [PATCH] datapath/linux: add KCFLAGS var to modules Makefile.main.in

2014-11-25 Thread Alexandru Ardelean
It's been a while since I made that patch.
I'll have to check.
I'll be back in a while with a few findings.

Thanks

On Tue, Nov 25, 2014 at 6:10 PM, Thomas Graf 
wrote:

> On 11/25/14 at 07:47am, Ben Pfaff wrote:
> > On Tue, Oct 28, 2014 at 07:51:16AM -0700, Ben Pfaff wrote:
> > > From: Alexandru Ardelean 
> > >
> > > This is mostly required because of GCC 4.9 which seems
> > > to error out with:
> > >   openvswitch/datapath/linux/datapath.c:2108:10:
> > >error: macro "DATE" might prevent reproducible builds
> > >
> > > We would have wanted to add '-Wno-error=date-time' directly
> > > but that would be too specific, so we decided to add
> > > a generic make flag and configure it with what we need.
> > >
> > > Signed-off-by: Alexandru Ardelean 
> > > ---
> > > This is from https://github.com/openvswitch/ovs/pull/18.  I'm posting
> it
> > > here for review because I'm not the right person to review it and I'm
> not
> > > sure that the OVS kernel maintainers are subscribed to the github
> issues
> > > tracker.
> >
> > Pravin or Jesse, would you mind take a look at this?
>
> Alexandru,
>
> You can already pass in additional build flags through the Kbuild
> EXTRA_CFLAGS:
>
> $ ./configure --with-linux=...
> $ cd datapath && make EXTRA_CFLAGS=-Wno-error=date-time
>
> Does that suit your needs?
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] tests: Fix test broken by introduction of ONFOXM_ET_TCP_FLAGS.

2014-11-25 Thread Ben Pfaff
Commit 847ddeab372220 (meta-flow: Add support for ONF OpenFlow 1.3
extension ONFOXM_ET_TCP_FLAGS.) failed to update a test to match the new
OXM extension.  This fixes the problem.

Signed-off-by: Ben Pfaff 
---
I already applied this as an obvious fix for the problem that I introduced
yesterday with the commit mentioned above.  Sorry for causing trouble!

diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
index eafaa63..9a5c71d 100644
--- a/tests/ovs-ofctl.at
+++ b/tests/ovs-ofctl.at
@@ -2340,14 +2340,20 @@ AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-nxm], [0],
   [NXM_OF_ETH_TYPE(0800), NXM_OF_IP_PROTO(06), NXM_NX_TCP_FLAGS(0fff)
 ])
 
-# Check that marshaling in OXM for OF1.2 through OF1.4 gives OXM
-# headers except for TCP flags, which didn't have an OXM definition until
-# OF1.5.
-for version in OpenFlow12 OpenFlow13 OpenFlow14; do
-AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm $version], [0],
-  [OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), NXM_NX_TCP_FLAGS(0fff)
+# Check that marshaling in OXM for OF1.2 gives OXM headers except for
+# TCP flags, which didn't have an OXM definition.
+AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow12], [0],
+[OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), NXM_NX_TCP_FLAGS(0fff)
+])
+
+# Check that marshaling in OXM for OF1.3 and OF1.4 gives OXM headers,
+# including the ONF extension for TCP flags introduced in OF1.3.
+AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow13], [0],
+[OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), ONFOXM_ET_TCP_FLAGS(0fff)
+])
+AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow14], [0],
+[OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), ONFOXM_ET_TCP_FLAGS(0fff)
 ])
-done
 
 # OpenFlow 1.5 added an OXM header for TCP flags:
 AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow15], [0],
-- 
2.1.3

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


Re: [ovs-dev] [PATCH 1/2] ofproto-dpif: add test case for OF1.4 packet-in

2014-11-25 Thread Ben Pfaff
On Fri, Nov 21, 2014 at 11:44:23AM -0800, Shu Shen wrote:
> The test case current fails and shows a bug when OF1.4 is used between
> the controller and the switch, the packet-in message still uses OF1.3
> wire protocol version.
> 
> Signed-off-by: Shu Shen 

Normally, I would not add a test before fixing the problem that it
checks for, but in this case Simon Horman has already fixed this
problem, so I applied this patch.  Thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows: Fixed Release broken build

2014-11-25 Thread Ben Pfaff
On Mon, Nov 24, 2014 at 04:26:00PM +, Sorin Vinturis wrote:
> The release configurations of the OVSEXT project were not compiling.
> This was due to a warning that was treated as error. Fixed that.
> 
> I did not want to remove that variable, because it is used in an
> ASSERT in that function.
> 
> Signed-off-by: Sorin Vinturis 

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


Re: [ovs-dev] [PATCH] datapath-windows: Update OVSEXT VS project to support 6.40

2014-11-25 Thread Ben Pfaff
On Mon, Nov 24, 2014 at 04:17:34PM +, Sorin Vinturis wrote:
> Added support for creating OVS extension driver NDIS 6.40 compliant.
> 
> Currently the OVSEXT Visual Studio project has four build configurations,
> 'Win8 Release', 'Win8 Debug', 'Win8.1 Release' and 'Win8.1 Debug'. All of
> them are creating a binary that is NDIS 6.30 compliant. I have changed the
> Win8.1 build configurations in order to create a binary that is NDIS 6.40
> compliant.
> 
> In this way, the OVSEXT project is able to create a release/debug binary
> that is NDIS 6.30 compliant, using the 'Win8 Release' and 'Win8 Debug'
> build configurations, as well as a release/debug binary that is NDIS 6.40
> compliant, using the 'Win8.1 Release' and 'Win8.1 Debug' build
> configurations.
> 
> Signed-off-by: Sorin Vinturis 

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


Re: [ovs-dev] [PATCH] ovs-ofctl.at: Fix test for TCP flags

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 05:13:10PM +0900, YAMAMOTO Takashi wrote:
> Update the test after commit 847ddeab372220170f431f1b9dd504237066729a
> ("meta-flow: Add support for ONF OpenFlow 1.3 extension ONFOXM_ET_TCP_FLAGS.")
> 
> Signed-off-by: YAMAMOTO Takashi 

Oops, sorry, I fixed this when I noticed it a few minutes ago.  I was
too embarrassed by the breakage to properly wait for review.  I
apologize; sorry to force you to duplicate my work.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows: Broken build on MSVC

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 03:52:28PM +, Alin Serdean wrote:
> Removing unused local variable.
> 
> 
> Signed-off-by: Alin Gabriel Serdean 

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


Re: [ovs-dev] [PATCH] Makefile: Add static check for misuse of LITTLE_ENDIAN and BIG_ENDIAN.

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 05:20:37PM +0100, Thomas Graf wrote:
> On 11/25/14 at 05:19pm, Thomas Graf wrote:
> > On 11/04/14 at 11:42am, Ben Pfaff wrote:
> > > I've pointed out two misuses of these macros in review in the last week,
> > > by different authors.  It's time to make it difficult to screw this up.
> > > 
> > > Signed-off-by: Ben Pfaff 
> > 
> > Missed s// above ^^^, should be check-endian
> 
> Otherwise:
> 
> Acked-by: Thomas Graf 

Thanks!  Applied, with the suggested change.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-vsctl: Prevent creating duplicate VLAN bridges.

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 05:25:56PM +0100, Thomas Graf wrote:
> On 11/06/14 at 02:59pm, Ben Pfaff wrote:
> > ovs-vsctl has the concept of a VLAN (or "fake") bridge, which is a
> > sort of a sub-bridge that receives only packets on a particular VLAN.
> > There is no way to distinguish two VLAN bridges with the same parent on the
> > same VLAN, but until now ovs-vsctl did not prevent creating duplicates or
> > report them.  This commit fixes the problem.
> > 
> > Reported-by: rwxybh
> > Reported-at: https://github.com/openvswitch/ovs/issues/21
> > Signed-off-by: Ben Pfaff 
> 
> Acked-by: Thomas Graf 

Thanks!  I'm rerunning the unit tests and I'll apply this to master
after they pass.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v3] datapath-windows: update DESIGN document

2014-11-25 Thread Nithin Raju
In this patch, we update the design document to reflect the netlink
based kernel-userspace interface implementation and a few other changes.
I have covered at a high level.

Please feel free to extend the document with more details that you think
got missed out.

Signed-off-by: Nithin Raju 
Acked-by: Sorin Vinturis 
---
 datapath-windows/DESIGN |  282 ++
 1 files changed, 184 insertions(+), 98 deletions(-)

diff --git a/datapath-windows/DESIGN b/datapath-windows/DESIGN
index b438c44..6d30adc 100644
--- a/datapath-windows/DESIGN
+++ b/datapath-windows/DESIGN
@@ -1,20 +1,13 @@
OVS-on-Hyper-V Design Document
==
-There has been an effort in the recent past to develop the Open vSwitch (OVS)
-solution onto multiple hypervisor platforms such as FreeBSD and Microsoft
-Hyper-V. VMware has been working on a OVS solution for Microsoft Hyper-V for
-the past few months and has successfully completed the implementation.
-
-This document provides details of the development effort. We believe this
-document should give enough information to members of the community who are
-curious about the developments of OVS on Hyper-V. The community should also be
-able to get enough information to make plans to leverage the deliverables of
-this effort.
-
-The userspace portion of the OVS has already been ported to Hyper-V and
-committed to the openvswitch repo. So, this document will mostly emphasize on
-the kernel driver, though we touch upon some of the aspects of userspace as
-well.
+There has been a community effort to develop Open vSwitch on Microsoft Hyper-V.
+In this document, we provide details of the development effort. We believe this
+document should give enough information to understand the overall design.
+
+The userspace portion of the OVS has been ported to Hyper-V in a separate
+effort, and committed to the openvswitch repo. So, this document will mostly
+emphasize on the kernel driver, though we touch upon some of the aspects of
+userspace as well.
 
 We cover the following topics:
 1. Background into relevant Hyper-V architecture
@@ -48,13 +41,13 @@ In Hyper-V, the virtual machine is called the Child 
Partition. Each VIF or
 physical NIC on the Hyper-V extensible switch is attached via a port. Each port
 is both on the ingress path or the egress path of the switch. The ingress path
 is used for packets being sent out of a port, and egress is used for packet
-being received on a port. By design, NDIS provides a layered interface, where
-in the ingress path, higher level layers call into lower level layers, and on
-the egress path, it is the other way round. In addition, there is a object
-identifier (OID) interface for control operations Eg. addition of a port. The
-workflow for the calls is similar in nature to the packets, where higher level
-layers call into the lower level layers. A good representational diagram of
-this architecture is in [4].
+being received on a port. By design, NDIS provides a layered interface. In this
+layered interface, higher level layers call into lower level layers, in the
+ingress path. In the egress path, it is the other way round. In addition, there
+is a object identifier (OID) interface for control operations Eg. addition of
+a port. The workflow for the calls is similar in nature to the packets, where
+higher level layers call into the lower level layers. A good representational
+diagram of this architecture is in [4].
 
 Windows Filtering Platform (WFP)[5] is a platform implemented on Hyper-V that
 provides APIs and services for filtering packets. WFP has been utilized to
@@ -75,22 +68,23 @@ has been used to retrieve some of the configuration 
information that OVS needs.
   |   |
   +--+ +--+   | +---+  ++ |
   |  | |  |   | |   |  || |
-  | OVS- | | OVS  |   | | Virtual   |  | Virtual| |
-  | wind | |  USERSPACE   |   | | Machine #1|  | Machine #2 | |
-  |  | |  DAEMON/CTL  |   | |   |  || |
+  | ovs- | | OVS- |   | | Virtual   |  | Virtual| |
+  | *ctl | |  USERSPACE   |   | | Machine #1|  | Machine #2 | |
+  |  | |DAEMON|   | |   |  || |
   +--+-++---+-+   | +--+--+-+  ++--++ | ++
-  |  DPIF-  |   | netdev- |   ||VIF #1| |VIF #2|  | |Physical|
-  | Windows |<=>| Windows |   |+--+ +--+  | |  NIC   |
+  |  dpif-  |   | netdev- |   ||VIF #1| |VIF #2|  | |Physical|
+  | netlink |   | windows |   |+--+ +--+  | |  NIC   |
   +-+   +-+   |  ||   /\  | ++
-User /\   |  || *#1* *#4* ||  | /\
-=||===+

Re: [ovs-dev] [PATCH v3] datapath-windows: update DESIGN document

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 09:06:43AM -0800, Nithin Raju wrote:
> In this patch, we update the design document to reflect the netlink
> based kernel-userspace interface implementation and a few other changes.
> I have covered at a high level.
> 
> Please feel free to extend the document with more details that you think
> got missed out.
> 
> Signed-off-by: Nithin Raju 
> Acked-by: Sorin Vinturis 

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


[ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

2014-11-25 Thread Alin Serdean
Add an additional check to see if the flowTable is not NULL.

kd> k
Child-SP  RetAddr   Call Site
d000`26166af8 f802`dde5e7c6 nt!DbgBreakPointWithStatus
d000`26166b00 f802`dde5e0d7 nt!KiBugCheckDebugBreak+0x12
d000`26166b60 f802`51a4 nt!KeBugCheck2+0x8ab
d000`26167270 f802`ddde0be9 nt!KeBugCheckEx+0x104
d000`261672b0 f802`f43a nt!KiBugCheckDispatch+0x69
d000`261673f0 f800`024cb4d4 nt!KiPageFault+0x23a
d000`26167580 f800`024cc3ef OVSExt!OvsDoDumpFlows+0xa0 
[c:\work\ovs\datapath-windows\ovsext\flow.c @ 2015]
d000`261675e0 f800`024d134c OVSExt!_FlowNlDumpCmdHandler+0x197 
[c:\work\ovs\datapath-windows\ovsext\flow.c @ 590]
d000`26167740 f800`024e128f OVSExt!InvokeNetlinkCmdHandler+0x6c 
[c:\work\ovs\datapath-windows\ovsext\datapath.c @ 952]
d000`26167770 f800`0053bc18 OVSExt!OvsDeviceControl+0x263 
[c:\work\ovs\datapath-windows\ovsext\datapath.c @ 862]
d000`26167840 f802`de04f395 NDIS!ndisDummyIrpHandler+0x88
d000`26167870 f802`de04fd2a nt!IopXxxControlFile+0x845
d000`26167a20 f802`ddde08b3 nt!NtDeviceIoControlFile+0x56
d000`26167a90 `775a2772 nt!KiSystemServiceCopyEnd+0x13
`009eee88 `775a2371 wow64cpu!CpupSyscallStub+0x2
`009eee90 `775c323a wow64cpu!DeviceIoctlFileFault+0x31
`009eef40 `775c317e wow64!RunCpuSimulation+0xa
`009eef90 7ffb`c1ca6bd0 wow64!Wow64LdrpInitialize+0x172
`009ef4d0 7ffb`c1ca6aa6 ntdll!_LdrpInitialize+0xd8
`009ef540 ` ntdll!LdrInitializeThunk+0xe
kd> !analyze -v
***
* *
*Bugcheck Analysis*
* *
***

DRIVER_IRQL_NOT_LESS_OR_EQUAL (d1)
An attempt was made to access a pageable (or completely invalid) address at an
interrupt request level (IRQL) that is too high.  This is usually
caused by drivers using improper addresses.
If kernel debugger is available get stack backtrace.
Arguments:
Arg1: , memory referenced
Arg2: 0002, IRQL
Arg3: , value 0 = read operation, 1 = write operation
Arg4: f800024cb4d4, address which referenced memory

Debugging Details:
--


"KERNEL32.DLL" was not found in the image list.
Debugger will attempt to load "KERNEL32.DLL" at given base `.

Please provide the full image name, including the extension (i.e. kernel32.dll)
for more reliable results.Base address and size overrides can be given as
.reload =,.
Unable to add module at `

READ_ADDRESS:  

CURRENT_IRQL:  2

FAULTING_IP:
OVSExt!OvsDoDumpFlows+a0 [c:\work\ovs\datapath-windows\ovsext\flow.c @ 2015]
f800`024cb4d4 488b18  mov rbx,qword ptr [rax]

DEFAULT_BUCKET_ID:  WIN8_DRIVER_FAULT

BUGCHECK_STR:  AV

PROCESS_NAME:  ovs-vswitchd.e

ANALYSIS_VERSION: 6.3.9600.17237 (debuggers(dbg).140716-0327) amd64fre

TRAP_FRAME:  d000261673f0 -- (.trap 0xd000261673f0)
NOTE: The trap frame does not contain all registers.
Some register values may be zeroed or incorrect.
rax= rbx= rcx=
rdx=d000261675e0 rsi= rdi=
rip=f800024cb4d4 rsp=d00026167580 rbp=
 r8=d00026167601  r9= r10=c00d
r11=d000261677b0 r12= r13=
r14= r15=
iopl=0 nv up ei pl zr na po nc
OVSExt!OvsDoDumpFlows+0xa0:
f800`024cb4d4 488b18  mov rbx,qword ptr [rax] 
ds:`=
Resetting default scope

LAST_CONTROL_TRANSFER:  from f802dde5e7c6 to f802bc90

STACK_TEXT:
d000`26166af8 f802`dde5e7c6 : ` ` 
d000`26166c60 f802`ddd83654 : nt!DbgBreakPointWithStatus
d000`26166b00 f802`dde5e0d7 : `0003 d000`26166c60 
f802`ddde3070 `00d1 : nt!KiBugCheckDebugBreak+0x12
d000`26166b60 f802`51a4 : ` `0001 
f6fb` d000`26e0 : nt!KeBugCheck2+0x8ab
d000`26167270 f802`ddde0be9 : `000a ` 
`0002 ` : nt!KeBugCheckEx+0x104
d000`261672b0 f802`f43a : ` ` 
e000`03cdbf00 d000`261673f0 : nt!KiBugCheckDispatch+0x69
d000`261673f0 f800`024cb4d4 : ` ` 
` ` : nt!KiPageFault+0x23a
d000`26167580 f800`024cc3ef : `00010300 ` 
`0002 e000`03e35e90 : OVS

Re: [ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

2014-11-25 Thread Sorin Vinturis
Acked-by: Sorin Vinturis 

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean
Sent: Tuesday, 25 November, 2014 19:09
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

Add an additional check to see if the flowTable is not NULL.

kd> k
Child-SP  RetAddr   Call Site
d000`26166af8 f802`dde5e7c6 nt!DbgBreakPointWithStatus
d000`26166b00 f802`dde5e0d7 nt!KiBugCheckDebugBreak+0x12
d000`26166b60 f802`51a4 nt!KeBugCheck2+0x8ab
d000`26167270 f802`ddde0be9 nt!KeBugCheckEx+0x104
d000`261672b0 f802`f43a nt!KiBugCheckDispatch+0x69
d000`261673f0 f800`024cb4d4 nt!KiPageFault+0x23a
d000`26167580 f800`024cc3ef OVSExt!OvsDoDumpFlows+0xa0 
[c:\work\ovs\datapath-windows\ovsext\flow.c @ 2015]
d000`261675e0 f800`024d134c OVSExt!_FlowNlDumpCmdHandler+0x197 
[c:\work\ovs\datapath-windows\ovsext\flow.c @ 590]
d000`26167740 f800`024e128f OVSExt!InvokeNetlinkCmdHandler+0x6c 
[c:\work\ovs\datapath-windows\ovsext\datapath.c @ 952]
d000`26167770 f800`0053bc18 OVSExt!OvsDeviceControl+0x263 
[c:\work\ovs\datapath-windows\ovsext\datapath.c @ 862]
d000`26167840 f802`de04f395 NDIS!ndisDummyIrpHandler+0x88
d000`26167870 f802`de04fd2a nt!IopXxxControlFile+0x845
d000`26167a20 f802`ddde08b3 nt!NtDeviceIoControlFile+0x56
d000`26167a90 `775a2772 nt!KiSystemServiceCopyEnd+0x13
`009eee88 `775a2371 wow64cpu!CpupSyscallStub+0x2
`009eee90 `775c323a wow64cpu!DeviceIoctlFileFault+0x31
`009eef40 `775c317e wow64!RunCpuSimulation+0xa
`009eef90 7ffb`c1ca6bd0 wow64!Wow64LdrpInitialize+0x172
`009ef4d0 7ffb`c1ca6aa6 ntdll!_LdrpInitialize+0xd8
`009ef540 ` ntdll!LdrInitializeThunk+0xe
kd> !analyze -v
***
* *
*Bugcheck Analysis*
* *
***

DRIVER_IRQL_NOT_LESS_OR_EQUAL (d1)
An attempt was made to access a pageable (or completely invalid) address at an 
interrupt request level (IRQL) that is too high.  This is usually caused by 
drivers using improper addresses.
If kernel debugger is available get stack backtrace.
Arguments:
Arg1: , memory referenced
Arg2: 0002, IRQL
Arg3: , value 0 = read operation, 1 = write operation
Arg4: f800024cb4d4, address which referenced memory

Debugging Details:
--


"KERNEL32.DLL" was not found in the image list.
Debugger will attempt to load "KERNEL32.DLL" at given base `.

Please provide the full image name, including the extension (i.e. kernel32.dll) 
for more reliable results.Base address and size overrides can be given as 
.reload =,.
Unable to add module at `

READ_ADDRESS:  

CURRENT_IRQL:  2

FAULTING_IP:
OVSExt!OvsDoDumpFlows+a0 [c:\work\ovs\datapath-windows\ovsext\flow.c @ 2015]
f800`024cb4d4 488b18  mov rbx,qword ptr [rax]

DEFAULT_BUCKET_ID:  WIN8_DRIVER_FAULT

BUGCHECK_STR:  AV

PROCESS_NAME:  ovs-vswitchd.e

ANALYSIS_VERSION: 6.3.9600.17237 (debuggers(dbg).140716-0327) amd64fre

TRAP_FRAME:  d000261673f0 -- (.trap 0xd000261673f0)
NOTE: The trap frame does not contain all registers.
Some register values may be zeroed or incorrect.
rax= rbx= rcx=
rdx=d000261675e0 rsi= rdi=
rip=f800024cb4d4 rsp=d00026167580 rbp=
 r8=d00026167601  r9= r10=c00d
r11=d000261677b0 r12= r13=
r14= r15=
iopl=0 nv up ei pl zr na po nc
OVSExt!OvsDoDumpFlows+0xa0:
f800`024cb4d4 488b18  mov rbx,qword ptr [rax] 
ds:`=
Resetting default scope

LAST_CONTROL_TRANSFER:  from f802dde5e7c6 to f802bc90

STACK_TEXT:
d000`26166af8 f802`dde5e7c6 : ` ` 
d000`26166c60 f802`ddd83654 : nt!DbgBreakPointWithStatus
d000`26166b00 f802`dde5e0d7 : `0003 d000`26166c60 
f802`ddde3070 `00d1 : nt!KiBugCheckDebugBreak+0x12
d000`26166b60 f802`51a4 : ` `0001 
f6fb` d000`26e0 : nt!KeBugCheck2+0x8ab
d000`26167270 f802`ddde0be9 : `000a ` 
`0002 ` : nt!KeBugCheckEx+0x104
d000`261672b0 f802`f43a : ` ` 
e000`03cdbf00 d000`261673f0 : nt!

Re: [ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

2014-11-25 Thread Eitan Eliahu

Hi Alin,
Thank you for fixing this issue. Do you think we can acquire the lock in 
OvsDeleteFlowTable() ?
Eitan

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Alin Serdean
Sent: Tuesday, November 25, 2014 9:09 AM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

Add an additional check to see if the flowTable is not NULL.

kd> k
Child-SP  RetAddr   Call Site
d000`26166af8 f802`dde5e7c6 nt!DbgBreakPointWithStatus
d000`26166b00 f802`dde5e0d7 nt!KiBugCheckDebugBreak+0x12
d000`26166b60 f802`51a4 nt!KeBugCheck2+0x8ab
d000`26167270 f802`ddde0be9 nt!KeBugCheckEx+0x104
d000`261672b0 f802`f43a nt!KiBugCheckDispatch+0x69
d000`261673f0 f800`024cb4d4 nt!KiPageFault+0x23a
d000`26167580 f800`024cc3ef OVSExt!OvsDoDumpFlows+0xa0 
[c:\work\ovs\datapath-windows\ovsext\flow.c @ 2015]
d000`261675e0 f800`024d134c OVSExt!_FlowNlDumpCmdHandler+0x197 
[c:\work\ovs\datapath-windows\ovsext\flow.c @ 590]
d000`26167740 f800`024e128f OVSExt!InvokeNetlinkCmdHandler+0x6c 
[c:\work\ovs\datapath-windows\ovsext\datapath.c @ 952]
d000`26167770 f800`0053bc18 OVSExt!OvsDeviceControl+0x263 
[c:\work\ovs\datapath-windows\ovsext\datapath.c @ 862]
d000`26167840 f802`de04f395 NDIS!ndisDummyIrpHandler+0x88
d000`26167870 f802`de04fd2a nt!IopXxxControlFile+0x845
d000`26167a20 f802`ddde08b3 nt!NtDeviceIoControlFile+0x56
d000`26167a90 `775a2772 nt!KiSystemServiceCopyEnd+0x13
`009eee88 `775a2371 wow64cpu!CpupSyscallStub+0x2
`009eee90 `775c323a wow64cpu!DeviceIoctlFileFault+0x31
`009eef40 `775c317e wow64!RunCpuSimulation+0xa
`009eef90 7ffb`c1ca6bd0 wow64!Wow64LdrpInitialize+0x172
`009ef4d0 7ffb`c1ca6aa6 ntdll!_LdrpInitialize+0xd8
`009ef540 ` ntdll!LdrInitializeThunk+0xe
kd> !analyze -v
***
* *
*Bugcheck Analysis*
* *
***

DRIVER_IRQL_NOT_LESS_OR_EQUAL (d1)
An attempt was made to access a pageable (or completely invalid) address at an 
interrupt request level (IRQL) that is too high.  This is usually caused by 
drivers using improper addresses.
If kernel debugger is available get stack backtrace.
Arguments:
Arg1: , memory referenced
Arg2: 0002, IRQL
Arg3: , value 0 = read operation, 1 = write operation
Arg4: f800024cb4d4, address which referenced memory

Debugging Details:
--


"KERNEL32.DLL" was not found in the image list.
Debugger will attempt to load "KERNEL32.DLL" at given base `.

Please provide the full image name, including the extension (i.e. kernel32.dll) 
for more reliable results.Base address and size overrides can be given as 
.reload =,.
Unable to add module at `

READ_ADDRESS:  

CURRENT_IRQL:  2

FAULTING_IP:
OVSExt!OvsDoDumpFlows+a0 [c:\work\ovs\datapath-windows\ovsext\flow.c @ 2015]
f800`024cb4d4 488b18  mov rbx,qword ptr [rax]

DEFAULT_BUCKET_ID:  WIN8_DRIVER_FAULT

BUGCHECK_STR:  AV

PROCESS_NAME:  ovs-vswitchd.e

ANALYSIS_VERSION: 6.3.9600.17237 (debuggers(dbg).140716-0327) amd64fre

TRAP_FRAME:  d000261673f0 -- (.trap 0xd000261673f0)
NOTE: The trap frame does not contain all registers.
Some register values may be zeroed or incorrect.
rax= rbx= rcx=
rdx=d000261675e0 rsi= rdi=
rip=f800024cb4d4 rsp=d00026167580 rbp=
 r8=d00026167601  r9= r10=c00d
r11=d000261677b0 r12= r13=
r14= r15=
iopl=0 nv up ei pl zr na po nc
OVSExt!OvsDoDumpFlows+0xa0:
f800`024cb4d4 488b18  mov rbx,qword ptr [rax] 
ds:`=
Resetting default scope

LAST_CONTROL_TRANSFER:  from f802dde5e7c6 to f802bc90

STACK_TEXT:
d000`26166af8 f802`dde5e7c6 : ` ` 
d000`26166c60 f802`ddd83654 : nt!DbgBreakPointWithStatus
d000`26166b00 f802`dde5e0d7 : `0003 d000`26166c60 
f802`ddde3070 `00d1 : nt!KiBugCheckDebugBreak+0x12
d000`26166b60 f802`51a4 : ` `0001 
f6fb` d000`26e0 : nt!KeBugCheck2+0x8ab
d000`26167270 f802`ddde0be9 : `000a ` 
`0002 ` : nt!KeBugCheckEx+0x104
d000`261672b0 f802`

Re: [ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

2014-11-25 Thread Nithin Raju
On Nov 25, 2014, at 9:09 AM, Alin Serdean 
 wrote:

> +if (datapath->flowTable == NULL) {
> +status = STATUS_INVALID_PARAMETER;
> +goto dp_unlock;
> +}

Alin,
We allocate the flow table in OvsAllocateFlowTable() when the datapath is 
allocated, which is when the 'gOvsSwitchContext' is also initialized.

The flow dump command handler which eventually calls into 
_FlowNlDumpCmdHandler() does check to see if 'gOvsSwitchContext' is valid or 
not since 'validateDpIndex' is TRUE in ValidateNetlinkCmd().

I was wondering how we'd end up exercising this code.

Thanks,
-- Nithin

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


Re: [ovs-dev] [PATCH] tests/ovs_client: Remove broken debug code

2014-11-25 Thread Pravin Shelar
On Wed, Nov 19, 2014 at 4:03 AM, Stephen Finucane
 wrote:
> There is some debug "logging" code included in the 'ovs_client'
> application. This code appears to be broken as it spams stdout with
> duplicated "log" messages. Remove this code.
>
> In addition, add the 'ovsclient' executable to the 'tests/.gitignore'
> file.
>
> Signed-off-by: Stephen Finucane 

I also renamed ovsclient to test-dpdkr and pushed patch to master.

Thanks.

> ---
>  tests/.gitignore  |  1 +
>  tests/ovs_client/ovs_client.c | 11 ---
>  2 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 908c50e..02fc822 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -7,6 +7,7 @@
>  /idltest.h
>  /idltest.ovsidl
>  /ovstest
> +/ovsclient
>  /ovs-pki.log
>  /pki/
>  /test-aes128
> diff --git a/tests/ovs_client/ovs_client.c b/tests/ovs_client/ovs_client.c
> index 6387624..97d32fc 100644
> --- a/tests/ovs_client/ovs_client.c
> +++ b/tests/ovs_client/ovs_client.c
> @@ -58,9 +58,6 @@
>   */
>  static uint8_t client_id = 0;
>
> -int no_pkt;
> -int pkt;
> -
>  /*
>   * Given the rx queue name template above, get the queue name.
>   */
> @@ -209,18 +206,10 @@ main(int argc, char *argv[])
>  }
>
>  if (rx_pkts > 0) {
> -pkt++;
>  /* blocking enqueue */
>  do {
>  rslt = rte_ring_enqueue_bulk(tx_ring, pkts, rx_pkts);
>  } while (rslt == -ENOBUFS);
> -} else {
> -   no_pkt++;
> -}
> -
> -if (!(pkt %  10)) {
> -printf("pkt %d %d\n", pkt, no_pkt);
> -pkt = no_pkt = 0;
>  }
>  }
>  }
> --
> 1.9.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] route-table: Remove Unregister.

2014-11-25 Thread Pravin B Shelar
Since dpif registering for routing table at initialization
there is no need to unregister it. Following patch removes
support for turning routing table notifications on and off.
Due to this change OVS always listens for these
notifications.

Reported-by: YAMAMOTO Takashi 
Signed-off-by: Pravin B Shelar 
---
 lib/netdev-vport.c   |1 -
 lib/route-table-bsd.c|   13 +-
 lib/route-table-stub.c   |5 
 lib/route-table.c|   53 --
 lib/route-table.h|1 -
 ofproto/ofproto-dpif-sflow.c |2 -
 6 files changed, 11 insertions(+), 64 deletions(-)

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 3825a09..e6b97fa 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -262,7 +262,6 @@ netdev_vport_destruct(struct netdev *netdev_)
 {
 struct netdev_vport *netdev = netdev_vport_cast(netdev_);
 
-route_table_unregister();
 free(netdev->peer);
 ovs_mutex_destroy(&netdev->mutex);
 }
diff --git a/lib/route-table-bsd.c b/lib/route-table-bsd.c
index 9ebfaa3..0d631b4 100644
--- a/lib/route-table-bsd.c
+++ b/lib/route-table-bsd.c
@@ -118,18 +118,7 @@ route_table_get_change_seq(void)
 void
 route_table_register(void)
 {
-if (!register_count)
-{
-pid = getpid();
-}
-
-register_count++;
-}
-
-void
-route_table_unregister(void)
-{
-register_count--;
+pid = getpid();
 }
 
 void
diff --git a/lib/route-table-stub.c b/lib/route-table-stub.c
index b7686e4..347149d 100644
--- a/lib/route-table-stub.c
+++ b/lib/route-table-stub.c
@@ -43,11 +43,6 @@ route_table_register(void)
 }
 
 void
-route_table_unregister(void)
-{
-}
-
-void
 route_table_run(void)
 {
 }
diff --git a/lib/route-table.c b/lib/route-table.c
index 63a9bd3..29ba38c 100644
--- a/lib/route-table.c
+++ b/lib/route-table.c
@@ -61,7 +61,6 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 
20);
  * every time route_table_reset() is called.  */
 static uint64_t rt_change_seq;
 
-static unsigned int register_count = 0;
 static struct nln *nln = NULL;
 static struct route_table_msg rtmsg;
 static struct nln_notifier *route_notifier = NULL;
@@ -76,7 +75,6 @@ static void route_table_change(const struct route_table_msg 
*, void *);
 static void route_map_clear(void);
 
 static void name_table_init(void);
-static void name_table_uninit(void);
 static void name_table_change(const struct rtnetlink_link_change *, void *);
 
 uint64_t
@@ -92,45 +90,20 @@ route_table_register(void)
 OVS_EXCLUDED(route_table_mutex)
 {
 ovs_mutex_lock(&route_table_mutex);
-if (!register_count) {
-ovs_assert(!nln);
-ovs_assert(!route_notifier);
+ovs_assert(!nln);
+ovs_assert(!route_notifier);
 
-ovs_router_init();
-nln = nln_create(NETLINK_ROUTE, RTNLGRP_IPV4_ROUTE,
- (nln_parse_func *) route_table_parse, &rtmsg);
+ovs_router_init();
+nln = nln_create(NETLINK_ROUTE, RTNLGRP_IPV4_ROUTE,
+ (nln_parse_func *) route_table_parse, &rtmsg);
 
-route_notifier =
-nln_notifier_create(nln, (nln_notify_func *) route_table_change,
-NULL);
+route_notifier =
+nln_notifier_create(nln, (nln_notify_func *) route_table_change,
+NULL);
 
-route_table_reset();
-name_table_init();
-}
-
-register_count++;
-ovs_mutex_unlock(&route_table_mutex);
-}
-
-/* Users of the route_table module should unregister themselves with this
- * function when they will no longer be making any more route_table fuction
- * calls. */
-void
-route_table_unregister(void)
-OVS_EXCLUDED(route_table_mutex)
-{
-ovs_mutex_lock(&route_table_mutex);
-register_count--;
+route_table_reset();
+name_table_init();
 
-if (!register_count) {
-nln_notifier_destroy(route_notifier);
-route_notifier = NULL;
-nln_destroy(nln);
-nln = NULL;
-
-route_map_clear();
-name_table_uninit();
-}
 ovs_mutex_unlock(&route_table_mutex);
 }
 
@@ -300,12 +273,6 @@ name_table_init(void)
 name_notifier = rtnetlink_link_notifier_create(name_table_change, NULL);
 }
 
-static void
-name_table_uninit(void)
-{
-rtnetlink_link_notifier_destroy(name_notifier);
-name_notifier = NULL;
-}
 
 static void
 name_table_change(const struct rtnetlink_link_change *change OVS_UNUSED,
diff --git a/lib/route-table.h b/lib/route-table.h
index 709dfb0..97f5b34 100644
--- a/lib/route-table.h
+++ b/lib/route-table.h
@@ -27,7 +27,6 @@
 
 uint64_t route_table_get_change_seq(void);
 void route_table_register(void);
-void route_table_unregister(void);
 void route_table_run(void);
 void route_table_wait(void);
 
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 60b65a3..35667ca 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -390,7 +390,6 @@ dpif_sflow_create(void)
 ds->n

Re: [ovs-dev] [PATCHv10 ovs 00/15] Revalidate flows with unique identifiers.

2014-11-25 Thread Joe Stringer
On 13 November 2014 at 11:17, Joe Stringer  wrote:
>
> This series modifies the dpif interface for flow commands to use 128-bit 
> unique
> identifiers as an alternative to the netlink-formatted flow key, and caches 
> the
> mask/actions in the udpif_key. This significantly reduces the cost of
> assembling messages between revalidators and the datapath, improving
> revalidation performance by 40% or more. In a test environment of many
> short-lived flows constantly being set up in the datapath, this increases the
> number of flows that can be maintained in the linux datapath from around
> 130-140K up to 190-200K. For the userspace datapath, this decreases the time
> spent revalidating 160K flows from 250ms to 150ms.
>
> The core of the changes sits in the handler and revalidator code. Handlers 
> take
> responsibility for creating udpif_key cache entries which now include a copy 
> of
> the flow mask and actions. Revalidators request datapaths to dump flows using
> only the unique identifier and stats, rather than the full set of
> netlink-formatted flow key, mask and actions.
>
> In cases where full revalidation is required, revalidators will use the
> udpif_key cache of the key/mask/acts to validate the flow. The dpif will
> detect datapath support for the unique identifer "UFID" feature, and omit flow
> keys from netlink transactions if it is supported. For backwards 
> compatibility,
> flow keys will always be serialised if UFID support is not detected in the
> datapath.
>
> Patches 1,2,3,15 are unreviewed. Patch 12 needs further review.
>
> This series is also made available here to assist review:
> https://github.com/joestringer/openvswitch/tree/submit/ufid_v10
>



>
> Joe Stringer (15):
>   tests: Add command to purge revalidators of flows.
>   ovs-bugtool: Log more detail for dumped flows.
>   datapath: Add 'is_mask' to ovs_nla_put_flow().
>   revalidator: Use 'cmap' for storing ukeys.
>   revalidator: Protect ukeys with a mutex.
>   udpif: Separate udpif_key maps from revalidators.
>   upcall: Rename dump_op -> ukey_op.
>   upcall: Create ukeys in handler threads.
>   upcall: Revalidate using cache of mask, actions.
>   hash: Add 128-bit murmurhash.
>   dpif: Generate flow_hash for revalidators in dpif.
>   datapath: Add support for unique flow identifiers.
>   dpif: Index flows using unique identifiers.
>   dpif: Minimize memory copy for revalidation.
>   dpctl: Add support for using UFID to add/del flows.




I'm currently addressing feedback for the datapath patches, which also
involves minor changes to patch #13.

I've already pushed patch #1 as it provided an unrelated benefit.  As
this series is fairly close, my current plan is to push patches 4-11
to master this afternoon; these are long-running unchanged patches. I
will then resend patches 2,13,14,15 and the openvswitch.h changes for
final review here. I'll rebase the datapath changes against net-next
and go through review there.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] vswitch.xml: Describe NetFlow passive timeout.

2014-11-25 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme 

On Nov 5, 2014, at 8:35 AM, Ben Pfaff  wrote:

> Reported-by: Martin Vizvary 
> Signed-off-by: Ben Pfaff 
> ---
> AUTHORS  |  1 +
> vswitchd/vswitch.xml | 18 ++
> 2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/AUTHORS b/AUTHORS
> index 1354703..2906708 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -257,6 +257,7 @@ Logan Rosen logatron...@gmail.com
> Luca Falavigna  dktrkr...@debian.org
> Luiz Henrique Ozaki luiz.oz...@gmail.com
> Marco d'Itrim...@linux.it
> +Martin Vizvary  vizv...@ics.muni.cz
> Maxime Brun m.b...@alphalink.fr
> Michael A. Collins  mike.a.coll...@ark-net.org
> Michael Hu  m...@nicira.com
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 0d78847..d02e5ff 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -3974,10 +3974,20 @@
> 
> 
> 
> -  The interval at which NetFlow records are sent for flows that are
> -  still active, in seconds.  A value of 0 requests the
> -  default timeout (currently 600 seconds); a value of -1
> -  disables active timeouts.
> +  
> + The interval at which NetFlow records are sent for flows that
> + are still active, in seconds.  A value of 0
> + requests the default timeout (currently 600 seconds); a value
> + of -1 disables active timeouts.
> +  
> +
> +  
> + The NetFlow passive timeout, for flows that become inactive,
> + is not configurable.  It will vary depending on the Open
> + vSwitch version, the forms and contents of the OpenFlow flow
> + tables, CPU and memory usage, and network activity.  A typical
> + passive timeout is about a second.
> +  
> 
> 
> 
> -- 
> 2.1.1
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev

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


Re: [ovs-dev] [PATCH] tests: Fix test broken by introduction of ONFOXM_ET_TCP_FLAGS.

2014-11-25 Thread Jarno Rajahalme
LGTM,

Acked-by: Jarno Rajahalme 

  Jarno

On Nov 25, 2014, at 8:39 AM, Ben Pfaff  wrote:

> Commit 847ddeab372220 (meta-flow: Add support for ONF OpenFlow 1.3
> extension ONFOXM_ET_TCP_FLAGS.) failed to update a test to match the new
> OXM extension.  This fixes the problem.
> 
> Signed-off-by: Ben Pfaff 
> ---
> I already applied this as an obvious fix for the problem that I introduced
> yesterday with the commit mentioned above.  Sorry for causing trouble!
> 
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index eafaa63..9a5c71d 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -2340,14 +2340,20 @@ AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-nxm], 
> [0],
>   [NXM_OF_ETH_TYPE(0800), NXM_OF_IP_PROTO(06), NXM_NX_TCP_FLAGS(0fff)
> ])
> 
> -# Check that marshaling in OXM for OF1.2 through OF1.4 gives OXM
> -# headers except for TCP flags, which didn't have an OXM definition until
> -# OF1.5.
> -for version in OpenFlow12 OpenFlow13 OpenFlow14; do
> -AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm $version], [0],
> -  [OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), NXM_NX_TCP_FLAGS(0fff)
> +# Check that marshaling in OXM for OF1.2 gives OXM headers except for
> +# TCP flags, which didn't have an OXM definition.
> +AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow12], [0],
> +[OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), NXM_NX_TCP_FLAGS(0fff)
> +])
> +
> +# Check that marshaling in OXM for OF1.3 and OF1.4 gives OXM headers,
> +# including the ONF extension for TCP flags introduced in OF1.3.
> +AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow13], [0],
> +[OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), ONFOXM_ET_TCP_FLAGS(0fff)
> +])
> +AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow14], [0],
> +[OXM_OF_ETH_TYPE(0800), OXM_OF_IP_PROTO(06), ONFOXM_ET_TCP_FLAGS(0fff)
> ])
> -done
> 
> # OpenFlow 1.5 added an OXM header for TCP flags:
> AT_CHECK([echo "$tcp_flags" | ovs-ofctl parse-oxm OpenFlow15], [0],
> -- 
> 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] vswitch.xml: Describe NetFlow passive timeout.

2014-11-25 Thread Ben Pfaff
Thanks! Applied to master.

On Tue, Nov 25, 2014 at 12:35:12PM -0800, Jarno Rajahalme wrote:
> Acked-by: Jarno Rajahalme 
> 
> On Nov 5, 2014, at 8:35 AM, Ben Pfaff  wrote:
> 
> > Reported-by: Martin Vizvary 
> > Signed-off-by: Ben Pfaff 
> > ---
> > AUTHORS  |  1 +
> > vswitchd/vswitch.xml | 18 ++
> > 2 files changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/AUTHORS b/AUTHORS
> > index 1354703..2906708 100644
> > --- a/AUTHORS
> > +++ b/AUTHORS
> > @@ -257,6 +257,7 @@ Logan Rosen logatron...@gmail.com
> > Luca Falavigna  dktrkr...@debian.org
> > Luiz Henrique Ozaki luiz.oz...@gmail.com
> > Marco d'Itrim...@linux.it
> > +Martin Vizvary  vizv...@ics.muni.cz
> > Maxime Brun m.b...@alphalink.fr
> > Michael A. Collins  mike.a.coll...@ark-net.org
> > Michael Hu  m...@nicira.com
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 0d78847..d02e5ff 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -3974,10 +3974,20 @@
> > 
> > 
> > 
> > -  The interval at which NetFlow records are sent for flows that are
> > -  still active, in seconds.  A value of 0 requests the
> > -  default timeout (currently 600 seconds); a value of -1
> > -  disables active timeouts.
> > +  
> > +   The interval at which NetFlow records are sent for flows that
> > +   are still active, in seconds.  A value of 0
> > +   requests the default timeout (currently 600 seconds); a value
> > +   of -1 disables active timeouts.
> > +  
> > +
> > +  
> > +   The NetFlow passive timeout, for flows that become inactive,
> > +   is not configurable.  It will vary depending on the Open
> > +   vSwitch version, the forms and contents of the OpenFlow flow
> > +   tables, CPU and memory usage, and network activity.  A typical
> > +   passive timeout is about a second.
> > +  
> > 
> > 
> > 
> > -- 
> > 2.1.1
> > 
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] route-table: Remove Unregister.

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 11:41:45AM -0800, Pravin B Shelar wrote:
> Since dpif registering for routing table at initialization
> there is no need to unregister it. Following patch removes
> support for turning routing table notifications on and off.
> Due to this change OVS always listens for these
> notifications.
> 
> Reported-by: YAMAMOTO Takashi 
> Signed-off-by: Pravin B Shelar 

Yamamoto-san, do you want to review this?

Thanks,

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


Re: [ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 05:09:29PM +, Alin Serdean wrote:
> Add an additional check to see if the flowTable is not NULL.

Hi Alin, can you provide a Signed-off-by?

Thanks,

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


Re: [ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

2014-11-25 Thread Nithin Raju
Hi Ben,
This change is not good to go. Alin has filed a bug for investigating this. 
Sorin or I will address it soon.

Thanks,
-- Nithin

- sent from mobile -

> On Nov 25, 2014, at 14:11, "Ben Pfaff"  wrote:
> 
>> On Tue, Nov 25, 2014 at 05:09:29PM +, Alin Serdean wrote:
>> Add an additional check to see if the flowTable is not NULL.
> 
> Hi Alin, can you provide a Signed-off-by?
> 
> Thanks,
> 
> Ben.
> ___
> dev mailing list
> dev@openvswitch.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=QTn6gqudM4fY-XUFwwhqbHxYFyO9IPwfYnOQALYtLP4&s=dB5o5zmHTo_VpM-jLBUrUiadkqkTeOgftuFv3TlJwt4&e=
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows: Fix BSOD when uninstalling driver

2014-11-25 Thread Ben Pfaff
OK, thanks for letting me know, I had the impression it was ready.

On Tue, Nov 25, 2014 at 10:15:19PM +, Nithin Raju wrote:
> Hi Ben,
> This change is not good to go. Alin has filed a bug for investigating this. 
> Sorin or I will address it soon.
> 
> Thanks,
> -- Nithin
> 
> - sent from mobile -
> 
> > On Nov 25, 2014, at 14:11, "Ben Pfaff"  wrote:
> > 
> >> On Tue, Nov 25, 2014 at 05:09:29PM +, Alin Serdean wrote:
> >> Add an additional check to see if the flowTable is not NULL.
> > 
> > Hi Alin, can you provide a Signed-off-by?
> > 
> > Thanks,
> > 
> > Ben.
> > ___
> > dev mailing list
> > dev@openvswitch.org
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=QTn6gqudM4fY-XUFwwhqbHxYFyO9IPwfYnOQALYtLP4&s=dB5o5zmHTo_VpM-jLBUrUiadkqkTeOgftuFv3TlJwt4&e=
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 1/2] lib: Add new header to versioning info

2014-11-25 Thread Ben Pfaff
On Mon, Nov 24, 2014 at 12:49:00PM +0100, Thomas Graf wrote:
> Exposes the package version as string and the library versioning
> as numeric macro.
> 
> OVS_LIB_VERSION may be used to check for a particular version of
> the interface.
> 
> OVS_LIB_REVISION and OVS_LIB_AGE exist to map a set of installed
> headers to a particular shared library instance.
> 
> Signed-off-by: Thomas Graf 

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


Re: [ovs-dev] [PATCH 2/2] lib: Add API to set program name and version

2014-11-25 Thread Ben Pfaff
On Mon, Nov 24, 2014 at 12:49:01PM +0100, Thomas Graf wrote:
> Required to have reasonable logging messages.
> 
> Signed-off-by: Thomas Graf 

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


Re: [ovs-dev] [PATCH] ofproto: fix checking of packet_in_mask in async config

2014-11-25 Thread Shu Shen
Hi Ben,

Have you got a chance to look at the below patch?

In the test case, wire_reason() translate OFPR_ACTION to OFPR_NO_MATCH, but 
OFPR_ACTION (as the value of pin->up.reason) was still passed to 
ofconn_receives_async_msg() to check whether the packet-in shall be sent or not 
- which it decides as not for the test case and thus the packet-in is not sent.

The patch instead passes the value from wire_reason() to 
ofconn_receives_async_msg() so that the async flag for NO_MATCH was tested and 
the packet-in will be sent for the test case.

Thanks,
Shu

-Original Message-
From: Shu Shen 
Sent: Friday, November 21, 2014 4:27 PM
To: dev@openvswitch.org
Cc: Shu Shen
Subject: [PATCH] ofproto: fix checking of packet_in_mask in async config

The check shall use wire protool reasons, which could be different from
the internal packet-in reason.

Signed-off-by: Shu Shen 
---
 ofproto/connmgr.c |  2 +-
 tests/ofproto-dpif.at | 62 +++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 627f326..46e7431 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1677,7 +1677,7 @@ connmgr_send_packet_in(struct connmgr *mgr,
 enum ofp_packet_in_reason reason = wire_reason(ofconn, pin);
 
 if (ofconn_wants_packet_in_on_miss(ofconn, pin)
-&& ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->up.reason)
+&& ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, reason)
 && ofconn->controller_id == pin->controller_id) {
 schedule_packet_in(ofconn, *pin, reason);
 }
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index d2d089b..684157b 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2652,6 +2652,68 @@ OFPST_FLOW reply (OF1.3):
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - table-miss flow with async config (OpenFlow 1.3)])
+OVS_VSWITCHD_START([dnl
+   add-port br0 p1 -- set Interface p1 type=dummy
+])
+ON_EXIT([kill `cat ovs-ofctl.pid`])
+
+AT_CAPTURE_FILE([ofctl_monitor.log])
+# A table-miss flow has priority 0 and no match
+AT_CHECK([ovs-ofctl --protocols=OpenFlow13 add-flow br0 'priority=0 
actions=output:CONTROLLER'])
+
+dnl Singleton controller action.
+AT_CHECK([ovs-ofctl monitor -P openflow10 --protocols=OpenFlow13 br0 65534 
--detach --no-chdir --pidfile 2> ofctl_monitor.log])
+
+# Become slave (OF 1.3), which should disable everything except port status.
+ovs-appctl -t ovs-ofctl ofctl/send 
04180018000200030001
+
+# Use OF 1.3 OFPT_SET_ASYNC to enable OFPR_NO_MATCH for slave only.
+ovs-appctl -t ovs-ofctl ofctl/send 
041c00220001
+
+for i in 1 2 3 ; do
+ovs-appctl netdev-dummy/receive p1 
'in_port(1),eth(src=10:11:11:11:11:11,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=10),tcp_flags(0x002)'
+done
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
+ovs-appctl -t ovs-ofctl exit
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+send: OFPT_ROLE_REQUEST (OF1.3) (xid=0x2): role=slave generation_id=1
+OFPT_ROLE_REPLY (OF1.3) (xid=0x2): role=slave generation_id=1
+dnl
+send: OFPT_SET_ASYNC (OF1.3) (xid=0x2):
+ master:
+   PACKET_IN: (off)
+ PORT_STATUS: (off)
+FLOW_REMOVED: (off)
+
+ slave:
+   PACKET_IN: no_match
+ PORT_STATUS: (off)
+FLOW_REMOVED: (off)
+dnl
+OFPT_PACKET_IN (OF1.3) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via 
no_match) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn
 tcp_csum:0
+dnl
+OFPT_PACKET_IN (OF1.3) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via 
no_match) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn
 tcp_csum:0
+dnl
+OFPT_PACKET_IN (OF1.3) (xid=0x0): cookie=0x0 total_len=60 in_port=1 (via 
no_match) data_len=60 (unbuffered)
+tcp,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10,tcp_flags=syn
 tcp_csum:0
+])
+
+AT_CHECK([ovs-appctl time/warp 5000], [0], [ignore])
+
+AT_CHECK([ovs-ofctl --protocols=OpenFlow13 dump-flows br0 | ofctl_strip | 
sort], [0], [dnl
+ n_packets=3, n_bytes=180, priority=0 actions=CONTROLLER:65535
+OFPST_FLOW reply (OF1.3):
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 
 AT_SETUP([ofproto-dpif - table-miss flow (OpenFlow 1.4)])
 OVS_VSWITCHD_START([dnl
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] ofproto: fix checking of packet_in_mask in async config

2014-11-25 Thread Ben Pfaff
On Fri, Nov 21, 2014 at 04:27:25PM -0800, Shu Shen wrote:
> The check shall use wire protool reasons, which could be different from
> the internal packet-in reason.
> 
> Signed-off-by: Shu Shen 

Thanks for finding the bug and for fixing it!  I applied this to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ofproto: fix checking of packet_in_mask in async config

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 11:18:38PM +, Shu Shen wrote:
> Have you got a chance to look at the below patch?

I was behind in review (I still am).  Thanks for the patch!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH/RFC 1/9] Add Netronome vendor Id

2014-11-25 Thread Ben Pfaff
On Wed, Nov 19, 2014 at 09:44:55AM +0900, Simon Horman wrote:
> Add Netronome vendor Id: NMX_VENDOR_ID = 0x1540.
> 
> This is based on the Netronome IEEE OUI, 00154D.
> And it has been registered with the ONF:
> 
> https://rs.opennetworking.org/wiki/display/PUBLIC/ONF+Registry
> 
> Signed-off-by: Simon Horman 

Applied, thanks.

For the record, there's no need to "register" OUIs with the ONF, since
namespace is reserved in the experimenter ID space for OUIs.  The page
you refer to does list some OUIs used as experimenter IDs, but that's
informational only, it's not a registry.

Thanks,

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


Re: [ovs-dev] [PATCH/RFC 0/9] Group Select: Selection Method Extension

2014-11-25 Thread Ben Pfaff
On Wed, Nov 19, 2014 at 09:44:54AM +0900, Simon Horman wrote:
> this patch set implements the group select selection method extension that
> I circulated some months ago on the d...@openvswtich.org mailing list.  For
> reference a copy of that proposal (updated for the existence of an Open
> Flow 1.5 draft and several errors found during implementation) is at the
> end of this email.
> 
> The implementation makes use of a group experimenter property and
> thus depends on my recent work to implement (draft) Open Flow 1.5
> groups (ONF EXT-250). That work is already present in the master branch.
> 
> The last patch of the series adds an implementation of a hash selection
> method to illustrate what a selection method might look like.  It may be
> thought of as a less intelligent but more flexible than the default
> selection method which I characterise as making hash of L2 and/or L3 fields
> depending on which fields are present in the flow.

Hi Simon.  Thanks for submitting this.

I have a question about the status of the definition of this
extension.  Is it an extension that is already in use, so that the OVS
implementation should strive to be compatible with the existing
implementation?  Or is it an extension for which this will be the
first implementation?  The answer colors my review, since in the first
case I'm mostly interested in faithfully implementing the extension in
an interoperable way, whereas in the second I'm also interested in
trying to ensure that it is defined in a way that is likely to be
useful and maintainable and extensible in the long run.

Thanks,

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


Re: [ovs-dev] [PATCH/RFC 0/9] Group Select: Selection Method Extension

2014-11-25 Thread Ben Pfaff
On another note, the description of the extension at the end of this
message is useful.  Does one of the patches add it to the tree
somewhere?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 01/10] flow.h: Simplify MAP_FOR_EACH_INDEX()

2014-11-25 Thread Ben Pfaff
On Thu, Nov 20, 2014 at 04:42:48PM -0800, Jarno Rajahalme wrote:
> Avoid comparing against specific values.
> 
> Signed-off-by: Jarno Rajahalme 

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


Re: [ovs-dev] [PATCH 02/10] flow: Define MINIFLOW_GET__() to simplify code.

2014-11-25 Thread Ben Pfaff
On Thu, Nov 20, 2014 at 04:42:49PM -0800, Jarno Rajahalme wrote:
> Signed-off-by: Jarno Rajahalme 

Can this be an inline function instead of a macro?  I'd prefer that.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 03/10] flow.c: Improve minimask_equal() and minimask_has_extra().

2014-11-25 Thread Ben Pfaff
On Thu, Nov 20, 2014 at 04:42:50PM -0800, Jarno Rajahalme wrote:
> minimask_equal() and minimask_has_extra() can take benefit from the
> fact that minimasks have no zero data.
> 
> Signed-off-by: Jarno Rajahalme 

I suspect that the new macro introduced in this patch could also be an
inline function.

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


[ovs-dev] [PATCH] dpif-netdev: Fix a race.

2014-11-25 Thread Alex Wang
On current master, the 'struct dp_netdev_port' is destroyed
immediately when the ref count reaches 0.  However, non-pmd
threads calling the dpif_netdev_execute() for sending packets
could hold pointer to 'port' that is not ref-counted.  Thusly
those threads could possibly access freed memory when the port
is deleted.

To fix this bug, this commit makes non-pmd threads acquiring
the 'port_mutex' before doing the actual execution in
dpif_netdev_execute().

Signed-off-by: Alex Wang 
---
 lib/dpif-netdev.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ea87023..5233130 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2029,10 +2029,12 @@ dpif_netdev_execute(struct dpif *dpif, struct 
dpif_execute *execute)
  * the 'non_pmd_mutex'. */
 if (pmd->core_id == NON_PMD_CORE_ID) {
 ovs_mutex_lock(&dp->non_pmd_mutex);
+ovs_mutex_lock(&dp->port_mutex);
 }
 dp_netdev_execute_actions(pmd, &pp, 1, false, execute->actions,
   execute->actions_len);
 if (pmd->core_id == NON_PMD_CORE_ID) {
+ovs_mutex_unlock(&dp->port_mutex);
 ovs_mutex_unlock(&dp->non_pmd_mutex);
 }
 
-- 
1.7.9.5

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


Re: [ovs-dev] [PATCH] dpif-netdev: Fix a race.

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 04:01:40PM -0800, Alex Wang wrote:
> On current master, the 'struct dp_netdev_port' is destroyed
> immediately when the ref count reaches 0.  However, non-pmd
> threads calling the dpif_netdev_execute() for sending packets
> could hold pointer to 'port' that is not ref-counted.  Thusly
> those threads could possibly access freed memory when the port
> is deleted.
> 
> To fix this bug, this commit makes non-pmd threads acquiring
> the 'port_mutex' before doing the actual execution in
> dpif_netdev_execute().
> 
> Signed-off-by: Alex Wang 

I don't know this code well enough to review it, but I hope that
there's a proper lock ordering discipline so that this fix doesn't
introduce a deadlock?

Thanks,

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


Re: [ovs-dev] [PATCH 04/10] lib: Use MAP_FOR_EACH_INDEX to improve readability.

2014-11-25 Thread Ben Pfaff
On Thu, Nov 20, 2014 at 04:42:51PM -0800, Jarno Rajahalme wrote:
> Signed-off-by: Jarno Rajahalme 

In most of the cases this just improves readability and should not
change the generated code much if at all.  In miniflow_hash(), though,
the previous code did not need to use raw_ctz(), which is relatively
expensive when no machine-specific implementation is available, but
the new version does use it.  Maybe that is OK, but it is a trade-off
that the other changes in this patch do not make.

Ditto for miniflow_equal().

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


[ovs-dev] [PATCH] ofproto: add support of OPRO_GROUP as packet-in reason for OF1.4+

2014-11-25 Thread Shu Shen
When the Packet-In message is triggered by a group action, OFPR_GROUP is
passed internally as the reason. The wire_reason() function converts the
reason to OFPR_ACTION if the wire protocol is earlier than OF1.4.

The wire_reason() function also converts other unsupported reasons
(i.e., OFPR_ACTION_SET and OFPR_PACKET_OUT) to OFPR_ACTION if it detects
a wire protocol earlier than OF1.4.

By default reason code OFPR_GROUP for Packet-In will be enabled for
async messages as in ofconn_flush(). Upon a connection being established
with a controller, the protocol version is checked and OFPR_GROUP will
be disabled in async config if the protocol is lower than OF1.4. Any
controller running OF1.4+ is still be able to enable OFPR_GROUP at its
will without being affected by this check.

The patch also includes tests cases for both OF1.3 and OF1.4 to ensure
proper reason code is given for packet-in message triggered by group
action.

Signed-off-by: Shu Shen 
---
 DESIGN.md|  1 +
 OPENFLOW-1.1+.md |  1 +
 ofproto/connmgr.c| 39 ++--
 ofproto/ofproto-dpif-xlate.c |  2 +-
 tests/ofproto-dpif.at| 88 
 5 files changed, 127 insertions(+), 4 deletions(-)

diff --git a/DESIGN.md b/DESIGN.md
index bd0ed27..ff5bdf4 100644
--- a/DESIGN.md
+++ b/DESIGN.md
@@ -54,6 +54,7 @@ sent, an entry labeled "---" means that the message is 
suppressed.
 OFPR_NO_MATCH  yes   ---
 OFPR_ACTIONyes   ---
 OFPR_INVALID_TTL   ---   ---
+OFPR_GROUP (OF1.4+)yes   ---
 
   OFPT_FLOW_REMOVED / NXT_FLOW_REMOVED
 OFPRR_IDLE_TIMEOUT yes   ---
diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md
index be28f16..967f906 100644
--- a/OPENFLOW-1.1+.md
+++ b/OPENFLOW-1.1+.md
@@ -197,6 +197,7 @@ OpenFlow 1.4 features are listed in the previous section.
   * More descriptive reasons for packet-in
 Distinguish OFPR_APPLY_ACTION, OFPR_ACTION_SET, OFPR_GROUP,
 OFPR_PACKET_OUT.  NO_MATCH was renamed to OFPR_TABLE_MISS.
+(OFPR_GROUP is now supported)
 [EXT-136]
 [required for OF1.4+]
 
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 46e7431..25a52fc 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -996,6 +996,14 @@ void
 ofconn_set_protocol(struct ofconn *ofconn, enum ofputil_protocol protocol)
 {
 ofconn->protocol = protocol;
+if (!(protocol & OFPUTIL_P_OF14_UP)) {
+uint32_t *master = ofconn->master_async_config;
+uint32_t *slave = ofconn->slave_async_config;
+
+/* OFPR_GROUP is not supported before OF1.4 */
+master[OAM_PACKET_IN] &= ~(1u << OFPR_GROUP);
+slave [OAM_PACKET_IN] &= ~(1u << OFPR_GROUP);
+}
 }
 
 /* Returns the currently configured packet in format for 'ofconn', one of
@@ -1056,6 +1064,13 @@ ofconn_get_async_config(struct ofconn *ofconn,
 uint32_t *master_masks, uint32_t *slave_masks)
 {
 size_t size = sizeof ofconn->master_async_config;
+
+/* Make sure we know the protocol version and the async_config
+ * masks are properly updated by calling ofconn_get_protocol() */
+if (OFPUTIL_P_NONE == ofconn_get_protocol(ofconn)){
+OVS_NOT_REACHED();
+}
+
 memcpy(master_masks, ofconn->master_async_config, size);
 memcpy(slave_masks, ofconn->slave_async_config, size);
 }
@@ -1235,7 +1250,9 @@ ofconn_flush(struct ofconn *ofconn)
 /* "master" and "other" roles get all asynchronous messages by default,
  * except that the controller needs to enable nonstandard "packet-in"
  * reasons itself. */
-master[OAM_PACKET_IN] = (1u << OFPR_NO_MATCH) | (1u << OFPR_ACTION);
+master[OAM_PACKET_IN] = ((1u << OFPR_NO_MATCH)
+ | (1u << OFPR_ACTION)
+ | (1u << OFPR_GROUP));
 master[OAM_PORT_STATUS] = ((1u << OFPPR_ADD)
| (1u << OFPPR_DELETE)
| (1u << OFPPR_MODIFY));
@@ -1651,16 +1668,32 @@ connmgr_send_flow_removed(struct connmgr *mgr,
 static enum ofp_packet_in_reason
 wire_reason(struct ofconn *ofconn, const struct ofproto_packet_in *pin)
 {
+enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
+
 if (pin->miss_type == OFPROTO_PACKET_IN_MISS_FLOW
 && pin->up.reason == OFPR_ACTION) {
-enum ofputil_protocol protocol = ofconn_get_protocol(ofconn);
 
 if (protocol != OFPUTIL_P_NONE
 && ofputil_protocol_to_ofp_version(protocol) >= OFP13_VERSION) {
 return OFPR_NO_MATCH;
 }
 }
-return pin->up.reason;
+
+switch (pin->up.reason) {
+case OFPR_ACTION_SET:
+case OFPR_GROUP:
+case OFPR_PACKET_OUT:
+   if (!(protocol & OFPUTIL_P_OF14_UP)) {
+/* Only supported in OF1

Re: [ovs-dev] [PATCH 05/10] lib: inline miniflow trivial functions.

2014-11-25 Thread Ben Pfaff
On Thu, Nov 20, 2014 at 04:42:52PM -0800, Jarno Rajahalme wrote:
> miniflow_clone_inline(), miniflow_destroy(), miniflow_expand(),
> miniflow_get(), miniflow_equal(), minimask_init(), minimask_clone(),
> minimask_move(), minimask_destroy(), minimask_expand(),
> minimask_expand, minimask_get(), minimask_equal(),
> minimask_has_extra(), minimatch_init(), minimatch_clone(),
> minimatch_move, minimatch_destroy(), minimatch_expand(),
> minimatch_equal(), and minimatch_matches_flow() are inlined.  Many of
> these call each other, so inlining could be beneficial.
> 
> miniflow_equal_in_minimask() is moved to lib/classifier, which is the
> sole user of it.
> 
> miniflow_equal_flow_in_minimask() is moved to tests/test-classifier,
> which is the only user of it.

Some of these definitions are fairly large.  I wonder whether it's
time to start making use of the GNU C "extern inline" feature,
described in the GCC manual:

 If you specify both `inline' and `extern' in the function
definition, then the definition is used only for inlining.  In no
case is the function compiled on its own, not even if you refer to
its address explicitly.  Such an address becomes an external
reference, as if you had only declared the function, and had not
defined it.

 This combination of `inline' and `extern' has almost the effect
of a macro.  The way to use it is to put a function definition in
a header file with these keywords, and put another copy of the
definition (lacking `inline' and `extern') in a library file.  The
definition in the header file will cause most calls to the
function to be inlined.  If any uses of the function remain, they
will refer to the single copy in the library.

What do you think?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] dpif-netdev: Fix a race.

2014-11-25 Thread Alex Wang
On Tue, Nov 25, 2014 at 4:02 PM, Ben Pfaff  wrote:

> On Tue, Nov 25, 2014 at 04:01:40PM -0800, Alex Wang wrote:
> > On current master, the 'struct dp_netdev_port' is destroyed
> > immediately when the ref count reaches 0.  However, non-pmd
> > threads calling the dpif_netdev_execute() for sending packets
> > could hold pointer to 'port' that is not ref-counted.  Thusly
> > those threads could possibly access freed memory when the port
> > is deleted.
> >
> > To fix this bug, this commit makes non-pmd threads acquiring
> > the 'port_mutex' before doing the actual execution in
> > dpif_netdev_execute().
> >
> > Signed-off-by: Alex Wang 
>
> I don't know this code well enough to review it, but I hope that
> there's a proper lock ordering discipline so that this fix doesn't
> introduce a deadlock?
>
>

Hey Ben,

Yeah, I discussed this issue with Pravin and found that using lock could be
more efficient than calling ovsrcu_synchronize.

I checked the code, and at this time, the "port_mutex" is only acquired for
port add/del/name query (only main thread deals with the mutex).  So, there
should be no double locking.

I'll ask Pravin for review,

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


[ovs-dev] [PATCH] dpif-netlink: Fix windows build.

2014-11-25 Thread Joe Stringer
Commit 7af12bd "dpif: Generate flow_hash for revalidators in dpif."
broke compilation on windows. Fix it.

Signed-off-by: Joe Stringer 
---
 lib/dpif-netlink.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 6c0cc07..f290cc6 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1977,7 +1977,7 @@ dpif_netlink_recv_windows(struct dpif_netlink *dpif, 
uint32_t handler_id,
 return error;
 }
 
-error = parse_odp_packet(buf, upcall, &dp_ifindex);
+error = parse_odp_packet(dpif, buf, upcall, &dp_ifindex);
 if (!error && dp_ifindex == dpif->dp_ifindex) {
 return 0;
 } else if (error) {
-- 
1.7.10.4

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


Re: [ovs-dev] [PATCH] dpif-netlink: Fix windows build.

2014-11-25 Thread Ben Pfaff
On Tue, Nov 25, 2014 at 04:39:49PM -0800, Joe Stringer wrote:
> Commit 7af12bd "dpif: Generate flow_hash for revalidators in dpif."
> broke compilation on windows. Fix it.
> 
> Signed-off-by: Joe Stringer 

Are you doing regular Windows builds?  That's nice, thanks.

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


Re: [ovs-dev] [PATCH] dpif-netlink: Fix windows build.

2014-11-25 Thread Joe Stringer
On 25 November 2014 at 16:44, Ben Pfaff  wrote:
> On Tue, Nov 25, 2014 at 04:39:49PM -0800, Joe Stringer wrote:
>> Commit 7af12bd "dpif: Generate flow_hash for revalidators in dpif."
>> broke compilation on windows. Fix it.
>>
>> Signed-off-by: Joe Stringer 
>
> Are you doing regular Windows builds?  That's nice, thanks.
>
> Acked-by: Ben Pfaff 

Nah, it just occurred to me that I might have broken it, so I investigated.

Thanks for the quick review, pushed to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCHv11 1/5] datapath: Add UFID interface to openvswitch.h.

2014-11-25 Thread Joe Stringer
An upcoming set of patches will implement support for indexing flows by
Unique Flow IDentifiers (UFID) rather than the traditional unmasked key.
This patch implements the interface changes required. The implementation
will follow.

Signed-off-by: Joe Stringer 
CC: Pravin B Shelar 
---
v11: Split from "datapath: Add support for unique flow identifiers."
---
 datapath/README.md|   13 +
 datapath/linux/compat/include/linux/openvswitch.h |   19 +++
 2 files changed, 32 insertions(+)

diff --git a/datapath/README.md b/datapath/README.md
index a8effa3..9c03a2b 100644
--- a/datapath/README.md
+++ b/datapath/README.md
@@ -131,6 +131,19 @@ performs best-effort detection of overlapping wildcarded 
flows and may reject
 some but not all of them. However, this behavior may change in future versions.
 
 
+Unique flow identifiers
+---
+
+An alternative to using the original match portion of a key as the handle for
+flow identification is a unique flow identifier, or "UFID". UFIDs are optional
+for both the kernel and user space program.
+
+User space programs that support UFID are expected to provide it during flow
+setup in addition to the flow, then refer to the flow using the UFID for all
+future operations. The kernel is not required to index flows by the original
+flow key if a UFID is specified.
+
+
 Basic rule for evolving flow keys
 -
 
diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
b/datapath/linux/compat/include/linux/openvswitch.h
index c8fa66e..67715f8 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -471,6 +471,13 @@ struct ovs_key_nd {
  * a wildcarded match. Omitting attribute is treated as wildcarding all
  * corresponding fields. Optional for all requests. If not present,
  * all flow key bits are exact match bits.
+ * @OVS_FLOW_ATTR_UFID: A unique identifier for the flow. Causes the flow to
+ * be indexed by this value rather than the %OVS_FLOW_ATTR_KEY%. Optional
+ * for all requests. Present in notifications if the flow was created with a
+ * UFID.
+ * @OVS_FLOW_ATTR_UFID_FLAGS: A 32-bit value of OR'd OVS_UFID_F_* flags that
+ * provide alternative semantics for flow installation and retrieval. Optional
+ * for all requests.
  *
  * These attributes follow the &struct ovs_header within the Generic Netlink
  * payload for %OVS_FLOW_* commands.
@@ -486,12 +493,24 @@ enum ovs_flow_attr {
OVS_FLOW_ATTR_MASK,  /* Sequence of OVS_KEY_ATTR_* attributes. */
OVS_FLOW_ATTR_PROBE, /* Flow operation is a feature probe, error
  * logging should be suppressed. */
+   OVS_FLOW_ATTR_UFID,  /* Variable length unique flow identifier. */
+   OVS_FLOW_ATTR_UFID_FLAGS,/* u32 of OVS_UFID_F_*. */
__OVS_FLOW_ATTR_MAX
 };
 
 #define OVS_FLOW_ATTR_MAX (__OVS_FLOW_ATTR_MAX - 1)
 
 /**
+ * Omit attributes for notifications.
+ *
+ * If a datapath request contains an OVS_UFID_F_OMIT_* flag, then the datapath
+ * may omit the corresponding 'ovs_flow_attr' from the response.
+ */
+#define OVS_UFID_F_OMIT_KEY  (1 << 0)
+#define OVS_UFID_F_OMIT_MASK (1 << 1)
+#define OVS_UFID_F_OMIT_ACTIONS  (1 << 2)
+
+/**
  * enum ovs_sample_attr - Attributes for %OVS_ACTION_ATTR_SAMPLE action.
  * @OVS_SAMPLE_ATTR_PROBABILITY: 32-bit fraction of packets to sample with
  * @OVS_ACTION_ATTR_SAMPLE.  A value of 0 samples no packets, a value of
-- 
1.7.10.4

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


[ovs-dev] [PATCHv11 0/5] Revalidate flows with unique identifiers.

2014-11-25 Thread Joe Stringer
=== Summary ===

Patches 1,4,5 are not yet reviewed. The linux datapath patches are not included
in this series, as I intend to rebase them against linux net-next and push them
through the review process on the netdev list.

This series is also made available here to assist review:
https://github.com/joestringer/openvswitch/tree/submit/ufid_v11

(Or with datapath changes as well, wip:)
https://github.com/joestringer/openvswitch/tree/submit/ufid_v11+datapath

=== Description ===

This series modifies the dpif interface for flow commands to use 128-bit unique
identifiers as an alternative to the netlink-formatted flow key, and caches the
mask/actions in the udpif_key. With datapath support, this significantly
reduces the cost of assembling messages between revalidators and the datapath,
improving revalidation performance by 40% or more. In a test environment of
many short-lived flows constantly being set up in the datapath, this increases
the number of flows that can be maintained in the linux datapath from around
130-140K up to 190-200K. For the userspace datapath, this decreases the time
spent revalidating 160K flows from 250ms to 150ms.

The core of the changes sits in the handler and revalidator code. Handlers take
responsibility for creating udpif_key cache entries which now include a copy of
the flow mask and actions. Revalidators request datapaths to dump flows using
only the unique identifier and stats, rather than the full set of
netlink-formatted flow key, mask and actions.

In cases where full revalidation is required, revalidators will use the
udpif_key cache of the key/mask/acts to validate the flow. The dpif will
detect datapath support for the unique identifer "UFID" feature, and omit flow
keys from netlink transactions if it is supported. For backwards compatibility,
flow keys will always be serialised if UFID support is not detected in the
datapath.

=== Changelog ===

v11:
- Pushed most of the prerequisite patches for this series to master.
- Split out openvswitch.h interface changes from datapath implementation
- Datapath implementation to be reviewed on net-next, separately
- Rebased

v10:
- New patch allowing datapath to serialize masked keys
- New patch providing commandline parsing of UFIDs
- New patch to fix IP fragment testsuite failure
- Simplify datapath interface by accepting UFID or flow_key, but not both
- Flows set up with UFID must be queried/deleted using UFID
- Reduce sw_flow memory usage for UFID
- Don't periodically rehash UFID table in linux datapath
- Remove kernel_only UFID in linux datapath
- Track whether UFIDs are present in datapath in udpif_key

v9:
- New patch to enable verbose flow-dumping in ovs-bugtool
- Don't print UFIDs by default in ovs-dpctl, ovs-appctl dump-flows output
- Userspace datapath performance and correctness improvements

v8:
- Rename UID -> UFID
- Clarify dpif interface descriptions
- Remove 'struct odputil_uidbuf'
- Simplify dpif-netlink UFID marshalling
- 32-bit build fix
- Fix null dereference in datapath when paired with older userspace
- Don't generate UFIDs for feature probes or ovs-dpctl usage
- Rebase
- All patches are reviewed/acked except datapath changes.

v7:
- Remove OVS_DP_F_INDEX_BY_UID
- Rework datapath UID serialization for variable length UIDs
- Create ukeys from revalidator threads in corner cases
- Hide "terse" flags from flow_get,flow_del dpif interface
- Scattered replacements of memcpy with u128_equal()
- Rebase

v6:
- Address feedback from Ben
- Split out "dpif: Add Unique flow identifiers." into three patches
- Reduce netlink conversions for all datapaths
- Reduce udpif_key footprint
- Added x64 version of murmurhash3
- Added hash function tests
- Various bugfixes
- Rebase

v5:
- Rebase
- Various bugfixes
- Improve logging

v4:
- Datapath memory leak fixes
- Enable UID-based terse dumping and deleting by default
- Shifted UID generation down to dpif
- Log flow UIDs in more places
- Various fixes

RFCv3:
- Add datapath implementation
- Minor fixes
- Rebased

RFCv2:
- Revised early patches from v1 feedback
- Add Acks from Ben
- Rebased

Joe Stringer (5):
  datapath: Add UFID interface to openvswitch.h.
  dpif: Index flows using unique identifiers.
  dpif: Minimize memory copy for revalidation.
  dpctl: Add support for using UFID to add/del flows.
  ovs-bugtool: Log more detail for dumped flows.

 datapath/README.md|   13 ++
 datapath/linux/compat/include/linux/openvswitch.h |   19 ++
 lib/dpctl.c   |   47 +++-
 lib/dpif-netdev.c |  156 --
 lib/dpif-netlink.c|  236 ++---
 lib/dpif-provider.h   |   13 +-
 lib/dpif.c|   50 -
 lib/dpif.h|   33 ++-
 lib/odp-util.c|   42 
 lib/odp-util.h

[ovs-dev] [PATCHv11 2/5] dpif: Index flows using unique identifiers.

2014-11-25 Thread Joe Stringer
This patch modifies the dpif interface to allow flows to be manipulated
using a 128-bit identifier. This allows revalidator threads to perform
datapath operations faster, as they do not need to serialise the entire
flow key for operations like flow_get and flow_delete. In conjunction
with a future patch to simplify the dump interface, this provides a
significant performance benefit for revalidation.

When handlers assemble flow_put operations, they specify a unique
identifier (UFID) for each flow as it is passed down to the datapath to
be stored with the flow. The UFID is currently provided to handlers
by the dpif during upcall processing.

When revalidators assemble flow_get or flow_del operations, they may
specify the UFID for the flow along with the key. The dpif will decide
whether to send only the UFID to the datapath, or both the UFID and flow
key. The former is preferred for newer datapaths that support UFID,
while the latter is used for backwards compatibility.

Signed-off-by: Joe Stringer 
Acked-by: Ben Pfaff 
---
v11: Switch to new UFID kernel interface.
v10: Adjust for datapath interface that accepts UFID or key as index.
 Pass "ufid_present" up to revalidators with dpif_flow.
v9: Fix flow lookup in dpif-netdev when UFID is missing.
Reduce hashing frequency in dpif-netdev with UFIDs.
Only print UFIDs in appctl/dpctl output when in verbose mode.
v8: Remove 'struct odputil_uidbuf'.
Allow flows in dpif-netdev to be looked up by flow key or ufid.
Don't generate UFIDs for feature probes (only for UFID feature probe).
Don't generate UFIDs in ovs-dpctl.
Clarify get/del interface description when ufid is invalid.
Replace memcpy with assignment in dp_netdev_flow_to_dpif_flow().
Rebase.
Add Ack-by.
v7: Shift UID below the dpif layer
Remove OVS_DP_F_INDEX_BY_UID
Use ovs_u128_equal().
Don't use 'static bool' for dpif_netlink_init_uid().
Rebase.
v6: Split out from "dpif: Add Unique flow identifiers."
Make sure that actions are fetched if terse=false.
Add additional dpif.h documentation to describe the interface.
Rebase.
v5: Always pass flow_key down to dpif, to improve error logging.
Fix extraneous netflow_unref.
Fix testsuite failure.
Rebase.
v4: Skip sending flow key in flow_del.
Fix race conditions with tests.
Combine dpif,upcall,dpif-netdev,dpif-linux changes into one patch.
Log UID in flow_del/flow_get/flow_dump messages.
v3: Rebase.
---
 lib/dpctl.c   |   12 ++-
 lib/dpif-netdev.c |   87 +++---
 lib/dpif-netlink.c|  199 -
 lib/dpif.c|   29 --
 lib/dpif.h|   30 +--
 lib/odp-util.c|7 ++
 lib/odp-util.h|1 +
 ofproto/ofproto-dpif-upcall.c |   23 +++--
 ofproto/ofproto-dpif.c|   12 ++-
 tests/dpif-netdev.at  |1 +
 tests/ofproto-dpif.at |   24 ++---
 tests/ofproto-macros.at   |1 +
 12 files changed, 333 insertions(+), 93 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 2d9144b..9adf9c8 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -771,6 +771,14 @@ dpctl_dump_flows(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 minimatch_destroy(&minimatch);
 }
 ds_clear(&ds);
+if (dpctl_p->verbosity) {
+if (f.ufid_present) {
+odp_format_ufid(&f.ufid, &ds);
+ds_put_cstr(&ds, ", ");
+} else {
+ds_put_cstr(&ds, "ufid:, ");
+}
+}
 odp_flow_format(f.key, f.key_len, f.mask, f.mask_len,
 &portno_names, &ds, dpctl_p->verbosity);
 ds_put_cstr(&ds, ", ");
@@ -852,7 +860,7 @@ dpctl_put_flow(int argc, const char *argv[], enum 
dpif_flow_put_flags flags,
   ofpbuf_size(&mask) == 0 ? NULL : ofpbuf_data(&mask),
   ofpbuf_size(&mask),
   ofpbuf_data(&actions), ofpbuf_size(&actions),
-  dpctl_p->print_statistics ? &stats : NULL);
+  NULL, dpctl_p->print_statistics ? &stats : NULL);
 if (error) {
 dpctl_error(dpctl_p, error, "updating flow table");
 goto out_freeactions;
@@ -938,7 +946,7 @@ dpctl_del_flow(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 }
 
 error = dpif_flow_del(dpif,
-  ofpbuf_data(&key), ofpbuf_size(&key),
+  ofpbuf_data(&key), ofpbuf_size(&key), NULL,
   dpctl_p->print_statistics ? &stats : NULL);
 if (error) {
 dpctl_error(dpctl_p, error, "deleting flow");
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ea87023..aa48474 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -304,6 +304,7 @@ struct dp_netdev_flow {
 
 /* Hash table index by unmasked flow. */
 const stru

[ovs-dev] [PATCHv11 5/5] ovs-bugtool: Log more detail for dumped flows.

2014-11-25 Thread Joe Stringer
The standard mode for printing flows doesn't always provide the full
range of information that is available, particularly with the UFID
changes. Turn on more detail.

Suggested-by: Justin Pettit 
Signed-off-by: Joe Stringer 
---
v10-v11: No change.
v9: First post.
---
 utilities/bugtool/ovs-bugtool-ovs-appctl-dpif |4 ++--
 utilities/bugtool/ovs-bugtool.in  |2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/utilities/bugtool/ovs-bugtool-ovs-appctl-dpif 
b/utilities/bugtool/ovs-bugtool-ovs-appctl-dpif
index 3560ef0..648ba68 100755
--- a/utilities/bugtool/ovs-bugtool-ovs-appctl-dpif
+++ b/utilities/bugtool/ovs-bugtool-ovs-appctl-dpif
@@ -20,7 +20,7 @@ for bridge in `ovs-vsctl -- --real list-br`
 do
 echo "ovs-appctl dpif/show ${bridge}"
 ovs-appctl dpif/show "${bridge}"
-echo "ovs-appctl dpif/dump-flows ${bridge}"
-ovs-appctl dpif/dump-flows "$bridge"
+echo "ovs-appctl dpif/dump-flows -m ${bridge}"
+ovs-appctl dpif/dump-flows -m "$bridge"
 echo ""
 done
diff --git a/utilities/bugtool/ovs-bugtool.in b/utilities/bugtool/ovs-bugtool.in
index 15f8271..75fcd0e 100755
--- a/utilities/bugtool/ovs-bugtool.in
+++ b/utilities/bugtool/ovs-bugtool.in
@@ -586,7 +586,7 @@ exclude those logs from the archive.
 if os.path.exists(OPENVSWITCH_VSWITCHD_PID):
 cmd_output(CAP_NETWORK_STATUS, [OVS_DPCTL, 'show', '-s'])
 for d in dp_list():
-cmd_output(CAP_NETWORK_STATUS, [OVS_DPCTL, 'dump-flows', d])
+cmd_output(CAP_NETWORK_STATUS, [OVS_DPCTL, 'dump-flows', '-m', d])
 
 cmd_output(CAP_PROCESS_LIST, [PS, 'wwwaxf', '-eo', 
'pid,tty,stat,time,nice,psr,pcpu,pmem,nwchan,wchan:25,args'], 
label='process-tree')
 func_output(CAP_PROCESS_LIST, 'fd_usage', fd_usage)
-- 
1.7.10.4

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


[ovs-dev] [PATCHv11 3/5] dpif: Minimize memory copy for revalidation.

2014-11-25 Thread Joe Stringer
One of the limiting factors on the number of flows that can be supported
in the datapath is the overhead of assembling flow dump messages in the
datapath. This patch modifies the dpif to allow revalidators to skip
dumping the key, mask and actions from the datapath, by making use of
the unique flow identifiers introduced in earlier patches.

For each flow dump, the dpif user specifies whether to skip these
attributes, allowing the common case to only dump a pair of 128-bit ID
and flow stats. With datapath support, this increases the number of
flows that a revalidator can handle per second by 50% or more.

Signed-off-by: Joe Stringer 
Acked-by: Ben Pfaff 
---
v11: Rebase.
v10: Ensure that UFIDs are only serialized to dpif-netlink when specified
   and UFID is supported in the datapath.
 Log UFID support from dpif-netlink later in ofproto_dpif init.
 Rebase.
v9: Fix some remaining references to "UID" rather than "UFID".
v8: Minor comment improvements.
Improve dpif-netlink flow serialization.
Rebase.
Added ack-by.
v7: Shift UID below the dpif layer.
Shift UID support detection to earlier patch.
Handle ukey_creation when key is missing from flow dump.
Use ovs_u128_equal().
v6: Split out from "dpif: Add Unique flow identifiers."
Rebase.
v5: Always pass flow_key down to dpif, to improve error logging.
Fix extraneous netflow_unref.
Rebase.
v4: Fix bug where UID-based terse dump wasn't enabled by default.
Fix race conditions with tests.
Combine dpif,upcall,dpif-netdev,dpif-linux changes into one patch.
Display whether terse dump is enabled in ovs-appctl upcall/show.
v3: Rebase.
---
 lib/dpctl.c   |2 +-
 lib/dpif-netdev.c |   71 --
 lib/dpif-netlink.c|   55 +++
 lib/dpif-provider.h   |   13 ++-
 lib/dpif.c|   21 +++-
 lib/dpif.h|7 +-
 ofproto/ofproto-dpif-upcall.c |  211 -
 ofproto/ofproto-dpif.c|2 +-
 tests/dpif-netdev.at  |4 +
 tests/ofproto-dpif.at |4 +
 10 files changed, 284 insertions(+), 106 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 9adf9c8..4e41fe4 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -747,7 +747,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 }
 
 ds_init(&ds);
-flow_dump = dpif_flow_dump_create(dpif);
+flow_dump = dpif_flow_dump_create(dpif, false);
 flow_dump_thread = dpif_flow_dump_thread_create(flow_dump);
 while (dpif_flow_dump_next(flow_dump_thread, &f, 1)) {
 if (filter) {
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index aa48474..44365cf 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1584,6 +1584,12 @@ get_dpif_flow_stats(const struct dp_netdev_flow 
*netdev_flow,
 }
 }
 
+static bool
+dpif_netdev_check_ufid(struct dpif *dpif OVS_UNUSED)
+{
+return true;
+}
+
 /* Converts to the dpif_flow format, using 'key_buf' and 'mask_buf' for
  * storing the netlink-formatted key/mask. 'key_buf' may be the same as
  * 'mask_buf'. Actions will be returned without copying, by relying on RCU to
@@ -1591,33 +1597,37 @@ get_dpif_flow_stats(const struct dp_netdev_flow 
*netdev_flow,
 static void
 dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
 struct ofpbuf *key_buf, struct ofpbuf *mask_buf,
-struct dpif_flow *flow)
+struct dpif_flow *flow, bool terse)
 {
-struct flow_wildcards wc;
-struct dp_netdev_actions *actions;
-size_t offset;
-
-miniflow_expand(&netdev_flow->cr.mask->mf, &wc.masks);
-
-/* Key */
-offset = ofpbuf_size(key_buf);
-flow->key = ofpbuf_tail(key_buf);
-odp_flow_key_from_flow(key_buf, &netdev_flow->flow, &wc.masks,
-   netdev_flow->flow.in_port.odp_port, true);
-flow->key_len = ofpbuf_size(key_buf) - offset;
-
-/* Mask */
-offset = ofpbuf_size(mask_buf);
-flow->mask = ofpbuf_tail(mask_buf);
-odp_flow_key_from_mask(mask_buf, &wc.masks, &netdev_flow->flow,
-   odp_to_u32(wc.masks.in_port.odp_port),
-   SIZE_MAX, true);
-flow->mask_len = ofpbuf_size(mask_buf) - offset;
-
-/* Actions */
-actions = dp_netdev_flow_get_actions(netdev_flow);
-flow->actions = actions->actions;
-flow->actions_len = actions->size;
+if (terse) {
+memset(flow, 0, sizeof *flow);
+} else {
+struct flow_wildcards wc;
+struct dp_netdev_actions *actions;
+size_t offset;
+
+miniflow_expand(&netdev_flow->cr.mask->mf, &wc.masks);
+
+/* Key */
+offset = ofpbuf_size(key_buf);
+flow->key = ofpbuf_tail(key_buf);
+odp_flow_key_from_flow(key_buf, &netdev_flow->flow, &wc.masks,
+   netdev_flow->flow.in_port.odp_port, true);
+  

Re: [ovs-dev] [PATCH 06/10] flow.c: Only support inline values in miniflow_extract().

2014-11-25 Thread Ben Pfaff
On Thu, Nov 20, 2014 at 04:42:53PM -0800, Jarno Rajahalme wrote:
> All the users of miniflow extract supply a miniflow with inlined data.
> Make the extraction a bit more efficient by only supporting this.
> 
> Also, clean up miniflow building utilities in preparation for moving
> them to lib/flow.h in a later patch.
> 
> Signed-off-by: Jarno Rajahalme 

Usually I just write a "module" name in the subject, e.g. "flow: Only
support inline values..."  I'm not sure that it's necessary to mention
that it's a .c or .h file.  (I see that a few patch subjects in this
series are written that way.)

This code for direct construction stuff is complex.  I wonder whether
a simpler approach would be just as fast and easier to understand.
Suppose that, instead of constructing a miniflow initially, we instead
construct a regular "struct flow" on the stack.  All the zeroing and
then later checking for nonzero values is what drove us earlier to
move to building a miniflow directly, so we'd want to avoid that.  But
we can do that by not initializing the flow at all, and just keeping
track in a map of the u32s we've initialized, and then copying those
fields into a miniflow based on the map we assembled.

I didn't read the details of the changes yet.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCHv11 4/5] dpctl: Add support for using UFID to add/del flows.

2014-11-25 Thread Joe Stringer
Signed-off-by: Joe Stringer 
---
XXX: Windows doesn't have a 'strndup' implementation, apparently. Should do
something about this.

v11: Rebase.
v10: First post.
---
 lib/dpctl.c|   41 +
 lib/odp-util.c |   35 +++
 lib/odp-util.h |1 +
 3 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 4e41fe4..7dc4714 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -819,9 +819,11 @@ dpctl_put_flow(int argc, const char *argv[], enum 
dpif_flow_put_flags flags,
 struct ofpbuf key;
 struct ofpbuf mask;
 struct dpif *dpif;
+ovs_u128 ufid;
+bool ufid_present;
 char *dp_name;
 struct simap port_names;
-int error;
+int n, error;
 
 dp_name = argc == 4 ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
 if (!dp_name) {
@@ -834,6 +836,15 @@ dpctl_put_flow(int argc, const char *argv[], enum 
dpif_flow_put_flags flags,
 return error;
 }
 
+ufid_present = false;
+n = odp_ufid_from_string(key_s, &ufid);
+if (n < 0) {
+dpctl_error(dpctl_p, error, "parsing flow ufid");
+return -n;
+} else if (n) {
+key_s += n;
+ufid_present = true;
+}
 
 simap_init(&port_names);
 DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
@@ -860,7 +871,8 @@ dpctl_put_flow(int argc, const char *argv[], enum 
dpif_flow_put_flags flags,
   ofpbuf_size(&mask) == 0 ? NULL : ofpbuf_data(&mask),
   ofpbuf_size(&mask),
   ofpbuf_data(&actions), ofpbuf_size(&actions),
-  NULL, dpctl_p->print_statistics ? &stats : NULL);
+  ufid_present ? &ufid : NULL,
+  dpctl_p->print_statistics ? &stats : NULL);
 if (error) {
 dpctl_error(dpctl_p, error, "updating flow table");
 goto out_freeactions;
@@ -916,9 +928,11 @@ dpctl_del_flow(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 struct ofpbuf key;
 struct ofpbuf mask; /* To be ignored. */
 struct dpif *dpif;
+ovs_u128 ufid;
+bool ufid_present;
 char *dp_name;
 struct simap port_names;
-int error;
+int n, error;
 
 dp_name = argc == 3 ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
 if (!dp_name) {
@@ -931,6 +945,16 @@ dpctl_del_flow(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 return error;
 }
 
+ufid_present = false;
+n = odp_ufid_from_string(key_s, &ufid);
+if (n < 0) {
+dpctl_error(dpctl_p, error, "parsing flow ufid");
+return -n;
+} else if (n) {
+key_s += n;
+ufid_present = true;
+}
+
 simap_init(&port_names);
 DPIF_PORT_FOR_EACH (&dpif_port, &port_dump, dpif) {
 simap_put(&port_names, dpif_port.name, odp_to_u32(dpif_port.port_no));
@@ -946,10 +970,19 @@ dpctl_del_flow(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
 }
 
 error = dpif_flow_del(dpif,
-  ofpbuf_data(&key), ofpbuf_size(&key), NULL,
+  ofpbuf_data(&key), ofpbuf_size(&key),
+  ufid_present ? &ufid : NULL,
   dpctl_p->print_statistics ? &stats : NULL);
 if (error) {
 dpctl_error(dpctl_p, error, "deleting flow");
+if (error == ENOENT && !ufid_present) {
+struct ds s;
+
+ds_init(&s);
+ds_put_format(&s, "Perhaps you need to specify a UFID?");
+dpctl_print(dpctl_p, "%s\n", ds_cstr(&s));
+ds_destroy(&s);
+}
 goto out;
 }
 
diff --git a/lib/odp-util.c b/lib/odp-util.c
index a89d5f8..647f26c 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1999,6 +1999,41 @@ generate_all_wildcard_mask(struct ofpbuf *ofp, const 
struct nlattr *key)
 return ofpbuf_base(ofp);
 }
 
+int
+odp_ufid_from_string(const char *s_, ovs_u128 *ufid)
+{
+const char *s = s_;
+
+if (!strncmp(s, "ufid:", 5)) {
+const char *upper;
+size_t n;
+
+s += 5;
+n = strspn(s, "0123456789abcdefABCDEF");
+if (!n || n > 32) {
+return -EINVAL;
+}
+
+upper = s;
+if (n > 16) {
+char *lower = strndup(s, n - 16);
+
+ufid->u64.lo = strtoull(lower, NULL, 16);
+upper += (n - 16);
+free(lower);
+} else {
+ufid->u64.lo = 0;
+}
+ufid->u64.hi = strtoull(upper, NULL, 16);
+s += n;
+s += strspn(s, delimiters);
+
+return s - s_;
+}
+
+return 0;
+}
+
 void
 odp_format_ufid(const ovs_u128 *ufid, struct ds *ds)
 {
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 27a5ca7..9c990cd 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -145,6 +145,7 @@ struct odputil_keybuf {
 enum odp_key_fitness odp_tun_key_from_attr(const struct nlattr *,
struc

Re: [ovs-dev] [PATCH/RFC 1/9] Add Netronome vendor Id

2014-11-25 Thread Simon Horman
On Tue, Nov 25, 2014 at 03:22:04PM -0800, Ben Pfaff wrote:
> On Wed, Nov 19, 2014 at 09:44:55AM +0900, Simon Horman wrote:
> > Add Netronome vendor Id: NMX_VENDOR_ID = 0x1540.
> > 
> > This is based on the Netronome IEEE OUI, 00154D.
> > And it has been registered with the ONF:
> > 
> > https://rs.opennetworking.org/wiki/display/PUBLIC/ONF+Registry
> > 
> > Signed-off-by: Simon Horman 
> 
> Applied, thanks.
> 
> For the record, there's no need to "register" OUIs with the ONF, since
> namespace is reserved in the experimenter ID space for OUIs.  The page
> you refer to does list some OUIs used as experimenter IDs, but that's
> informational only, it's not a registry.

Thanks for clarifying that, I was under the impression that it was
a registry.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH/RFC 0/9] Group Select: Selection Method Extension

2014-11-25 Thread Simon Horman
On Tue, Nov 25, 2014 at 03:30:01PM -0800, Ben Pfaff wrote:
> On Wed, Nov 19, 2014 at 09:44:54AM +0900, Simon Horman wrote:
> > this patch set implements the group select selection method extension that
> > I circulated some months ago on the d...@openvswtich.org mailing list.  For
> > reference a copy of that proposal (updated for the existence of an Open
> > Flow 1.5 draft and several errors found during implementation) is at the
> > end of this email.
> > 
> > The implementation makes use of a group experimenter property and
> > thus depends on my recent work to implement (draft) Open Flow 1.5
> > groups (ONF EXT-250). That work is already present in the master branch.
> > 
> > The last patch of the series adds an implementation of a hash selection
> > method to illustrate what a selection method might look like.  It may be
> > thought of as a less intelligent but more flexible than the default
> > selection method which I characterise as making hash of L2 and/or L3 fields
> > depending on which fields are present in the flow.
> 
> Hi Simon.  Thanks for submitting this.
> 
> I have a question about the status of the definition of this
> extension.  Is it an extension that is already in use, so that the OVS
> implementation should strive to be compatible with the existing
> implementation?  Or is it an extension for which this will be the
> first implementation?  The answer colors my review, since in the first
> case I'm mostly interested in faithfully implementing the extension in
> an interoperable way, whereas in the second I'm also interested in
> trying to ensure that it is defined in a way that is likely to be
> useful and maintainable and extensible in the long run.

It is the latter: this is the only implementation I am aware of
and I am not aware of any users in the wild.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH/RFC 0/9] Group Select: Selection Method Extension

2014-11-25 Thread Simon Horman
On Tue, Nov 25, 2014 at 03:31:50PM -0800, Ben Pfaff wrote:
> On another note, the description of the extension at the end of this
> message is useful.  Does one of the patches add it to the tree
> somewhere?

No, but I'm happy to add such a patch.

It could go into a header or .c file somewhat but perhaps
documentation/group-selection-method.txt is a better idea.

Let me know what you think and I will make it so.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 2/2] ovs-command-completion: Autotest integration.

2014-11-25 Thread Alex Wang
This commit integrates the unit tests defined in
utilities/ovs-command-compgen-test.bash into 'make check'.
The tests will be skipped if the current shell is not
'/bin/bash'.

Signed-off-by: Alex Wang 
---
 Makefile.am |1 +
 tests/automake.mk   |1 +
 tests/completion.at |  354 
 tests/testsuite.at  |1 +
 utilities/automake.mk   |3 +-
 utilities/ovs-command-compgen-test.bash |  688 ---
 6 files changed, 359 insertions(+), 689 deletions(-)
 create mode 100644 tests/completion.at
 delete mode 100755 utilities/ovs-command-compgen-test.bash

diff --git a/Makefile.am b/Makefile.am
index ddc7acb..fe4e592 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -126,6 +126,7 @@ scripts_SCRIPTS =
 scripts_DATA =
 SUFFIXES =
 check_DATA =
+check_SCRIPTS =
 pkgconfig_DATA =
 
 scriptsdir = $(pkgdatadir)/scripts
diff --git a/tests/automake.mk b/tests/automake.mk
index ccce112..b5a2d3d 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -6,6 +6,7 @@ EXTRA_DIST += \
$(srcdir)/tests/testsuite
 TESTSUITE_AT = \
tests/testsuite.at \
+   tests/completion.at \
tests/ovsdb-macros.at \
tests/library.at \
tests/heap.at \
diff --git a/tests/completion.at b/tests/completion.at
new file mode 100644
index 000..7095ff3
--- /dev/null
+++ b/tests/completion.at
@@ -0,0 +1,354 @@
+AT_BANNER([command completion unit tests - bash])
+
+m4_define([GET_FORMAT], [
+echo "$@" | grep -A 1 -- "Command format" | tail -n+2
+])
+
+m4_define([GET_EXPAN], [
+echo "$@" | grep -- "available completions for keyword" \
+ | sed -e 's/^[ ]*//g;s/[ ]*$//g'
+])
+
+m4_define([GET_AVAIL], [
+echo "$@" | sed -e '1,/Available/d' | tail -n+2
+])
+
+m4_define([GET_COMP_STR], [
+echo "available completions for keyword \"$1\": $2" \
+ | sed -e 's/[ ]*$//g'
+])
+
+AT_SETUP([bash completion - basic verification])
+AT_SKIP_IF([test "$(echo $SHELL)" != "/bin/bash"])
+OVS_VSWITCHD_START
+
+# complete ovs-appctl [TAB]
+# complete ovs-dpctl  [TAB]
+# complete ovs-ofctl  [TAB]
+# complete ovsdb-tool [TAB]
+m4_foreach(
+[test_command],
+[[ovs-appctl],
+[ovs-dpctl],
+[ovs-ofctl],
+[ovsdb-tool]],
+[
+INPUT="$(bash ovs-command-compgen.bash debug test_command TAB 2>&1)"
+MATCH="$(test_command --option | sort | sed -n '/^--.*/p' | cut -d '=' -f1)
+$(test_command list-commands | tail -n +2 | cut -c3- | cut -d ' ' -f1 | sort)"
+AT_CHECK_UNQUOTED([GET_AVAIL(${INPUT})],
+[0], [dnl
+${MATCH}
+])])
+
+
+# complete ovs-appctl --tar[TAB]
+INPUT="$(bash ovs-command-compgen.bash debug ovs-appctl --tar 2>&1)"
+AT_CHECK_UNQUOTED([GET_AVAIL(${INPUT})],
+[0], [dnl
+--target
+])
+
+
+# complete ovs-appctl --target [TAB]
+INPUT="$(bash ovs-command-compgen.bash debug ovs-appctl --target TAB 2>&1)"
+AT_CHECK_UNQUOTED([GET_AVAIL(${INPUT})],
+[0], [dnl
+ovs-ofctl
+ovs-vswitchd
+ovsdb-server
+])
+
+
+# complete ovs-appctl --target ovs-vswitchd [TAB]
+# complete ovs-appctl --target ovsdb-server [TAB]
+# complete ovs-appctl --target ovs-ofctl[TAB]
+AT_CHECK([ovs-ofctl monitor br0 --detach --no-chdir --pidfile])
+m4_foreach(
+[target_daemon],
+[[ovs-vswitchd],
+[ovsdb-server],
+[ovs-ofctl]],
+[
+INPUT="$(bash ovs-command-compgen.bash debug ovs-appctl --target target_daemon 
TAB 2>&1)"
+MATCH="$(ovs-appctl --option | sort | sed -n '/^--.*/p' | cut -d '=' -f1)
+$(ovs-appctl --target target_daemon list-commands | tail -n +2 | cut -c3- | 
cut -d ' ' -f1 | sort)"
+AT_CHECK_UNQUOTED([GET_AVAIL(${INPUT})],
+[0], [dnl
+${MATCH}
+])])
+AT_CHECK([ovs-appctl --target ovs-ofctl exit])
+
+
+# check all ovs-appctl subcommand formats
+LIST=$(ovs-appctl list-commands | tail -n +2 | cut -c3- | cut -d ' ' -f1 | 
sort)
+for subcommand in $LIST; do
+INPUT="$(bash ovs-command-compgen.bash debug ovs-appctl $subcommand TAB 
2>&1)"
+MATCH=$(ovs-appctl list-commands | tail -n+2 | cut -c3- | grep -- 
"^$subcommand " | tr -s ' ')
+AT_CHECK_UNQUOTED([GET_FORMAT(${INPUT})],
+[0], [dnl
+${MATCH}
+])
+done
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+
+# complex completion check - bfd/set-forwarding
+# bfd/set-forwarding [interface] normal|false|true
+# test expansion of 'interface'
+AT_SETUP([bash completion - complex completion check 1])
+AT_SKIP_IF([test "$(echo $SHELL)" != "/bin/bash"])
+OVS_VSWITCHD_START(add-port br0 p0 -- set Interface p0 type=dummy)
+
+# check the top level completion.
+INPUT="$(bash ovs-command-compgen.bash debug ovs-appctl bfd/set-forwarding TAB 
2>&1)"
+MATCH="$(GET_COMP_STR([normal], [])
+GET_COMP_STR([false], [])
+GET_COMP_STR([true], [])
+GET_COMP_STR([interface], [p0]))"
+AT_CHECK_UNQUOTED([GET_EXPAN(${INPUT})],
+[0], [dnl
+${MATCH}
+])
+# check the available completions.
+AT_CHECK_UNQUOTED([GET_AVAIL(${INPUT})],
+[0], [dnl
+p0
+])
+
+
+# set argument to 'true', there should be no more completions.
+INPUT="$(bash ovs-command-compgen.bash debug ovs-appctl bfd/set-forwarding 
true TAB 2

[ovs-dev] [PATCH 1/2] ovs-command-completion: Fix unwanted whitespace.

2014-11-25 Thread Alex Wang
This commit fixes unwanted whitespace in the ovs
bash completion script output.

Signed-off-by: Alex Wang 
---
 utilities/ovs-command-compgen.bash |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utilities/ovs-command-compgen.bash 
b/utilities/ovs-command-compgen.bash
index c87445a..a515c1d 100755
--- a/utilities/ovs-command-compgen.bash
+++ b/utilities/ovs-command-compgen.bash
@@ -371,7 +371,7 @@ kwords_to_args() {
 match=
 ;;
 esac
-match=$(echo "$match" | tr '\n' ' ' | sed -e 's/^[ \t]*//')
+match=$(echo "$match" | tr '\n' ' ' | tr -s ' ' | sed -e 's/^[ \t]*//')
 args+=( $match )
 if [ -n "$_PRINTF_ENABLE" ]; then
 local output_stderr=
-- 
1.7.9.5

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


Re: [ovs-dev] [PATCH] route-table: Remove Unregister.

2014-11-25 Thread YAMAMOTO Takashi
> On Tue, Nov 25, 2014 at 11:41:45AM -0800, Pravin B Shelar wrote:
>> Since dpif registering for routing table at initialization
>> there is no need to unregister it. Following patch removes
>> support for turning routing table notifications on and off.
>> Due to this change OVS always listens for these
>> notifications.
>> 
>> Reported-by: YAMAMOTO Takashi 
>> Signed-off-by: Pravin B Shelar 
> 
> Yamamoto-san, do you want to review this?

i'll take a look later today.

YAMAMOTO Takashi

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


Re: [ovs-dev] [PATCH] lib/route-table-*: Fix non-Linux builds

2014-11-25 Thread YAMAMOTO Takashi
> On Tue, Nov 25, 2014 at 05:01:13PM +0900, YAMAMOTO Takashi wrote:
>> Fix build failures introduced by
>> commit 1bc50ef389d40be9ee215e66b49cf66fcbdb9eeb
>> ("dpctl: Fix crash.")
>> 
>> Signed-off-by: YAMAMOTO Takashi 
> 
> Acked-by: Ben Pfaff 

thank you.  applied.

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


Re: [ovs-dev] [PATCH] route-table: Remove Unregister.

2014-11-25 Thread Ben Pfaff
On Wed, Nov 26, 2014 at 11:20:54AM +0900, YAMAMOTO Takashi wrote:
> > On Tue, Nov 25, 2014 at 11:41:45AM -0800, Pravin B Shelar wrote:
> >> Since dpif registering for routing table at initialization
> >> there is no need to unregister it. Following patch removes
> >> support for turning routing table notifications on and off.
> >> Due to this change OVS always listens for these
> >> notifications.
> >> 
> >> Reported-by: YAMAMOTO Takashi 
> >> Signed-off-by: Pravin B Shelar 
> > 
> > Yamamoto-san, do you want to review this?
> 
> i'll take a look later today.

Thanks!

By the way, the U.S.-based developers (me, and most of us) are likely to
be less responsive for the rest of this week, and especially on Thursday
and Friday, because of the "Thanksgiving" holiday in the U.S.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-ofctl.at: Fix test for TCP flags

2014-11-25 Thread YAMAMOTO Takashi
> On Tue, Nov 25, 2014 at 05:13:10PM +0900, YAMAMOTO Takashi wrote:
>> Update the test after commit 847ddeab372220170f431f1b9dd504237066729a
>> ("meta-flow: Add support for ONF OpenFlow 1.3 extension 
>> ONFOXM_ET_TCP_FLAGS.")
>> 
>> Signed-off-by: YAMAMOTO Takashi 
> 
> Oops, sorry, I fixed this when I noticed it a few minutes ago.  I was
> too embarrassed by the breakage to properly wait for review.  I
> apologize; sorry to force you to duplicate my work.

no problem!

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


Re: [ovs-dev] [PATCH/RFC 1/9] Add Netronome vendor Id

2014-11-25 Thread Ben Pfaff
On Wed, Nov 26, 2014 at 10:33:29AM +0900, Simon Horman wrote:
> On Tue, Nov 25, 2014 at 03:22:04PM -0800, Ben Pfaff wrote:
> > On Wed, Nov 19, 2014 at 09:44:55AM +0900, Simon Horman wrote:
> > > Add Netronome vendor Id: NMX_VENDOR_ID = 0x1540.
> > > 
> > > This is based on the Netronome IEEE OUI, 00154D.
> > > And it has been registered with the ONF:
> > > 
> > > https://rs.opennetworking.org/wiki/display/PUBLIC/ONF+Registry
> > > 
> > > Signed-off-by: Simon Horman 
> > 
> > Applied, thanks.
> > 
> > For the record, there's no need to "register" OUIs with the ONF, since
> > namespace is reserved in the experimenter ID space for OUIs.  The page
> > you refer to does list some OUIs used as experimenter IDs, but that's
> > informational only, it's not a registry.
> 
> Thanks for clarifying that, I was under the impression that it was
> a registry.

There *is* a registry on that page, but it's separate from the list of
OUIs.  A company (or individual) only needs to add an entry to the
registry if it does not own an OUI.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v5 11/13] datapath: Allow masks for set actions.

2014-11-25 Thread Jesse Gross
On Fri, Sep 5, 2014 at 4:05 PM, Jarno Rajahalme  wrote:
> Masked set action allows more megaflow wildcarding.  Masked set action
> is now supported for all writeable key types, except for the tunnel
> key.
>
> The set tunnel action is an exception as any input tunnel info is
> cleared before action processing starts, so there is no tunnel info to
> mask.
>
> The kernel module converts all (non-tunnel) set actions to masked set
> actions.  This makes action processing more uniform, and results in
> less branching and duplicating the action processing code.  When
> returning actions to userspace, the fully masked set actions are
> converted to normal set actions.  To make this mapping consistent, we
> reject fully masked set actions at flow set-up time.
>
> Signed-off-by: Jarno Rajahalme 

Sorry about the extremely long delay, I'm finally coming back to this...

> diff --git a/datapath/actions.c b/datapath/actions.c
> index 43ca2a0..243b672 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> +static int set_ipv6(struct sk_buff *skb, const struct ovs_key_ipv6 *key,
> +   const struct ovs_key_ipv6 *mask)
>  {
> @@ -483,38 +525,56 @@ static int set_ipv6(struct sk_buff *skb, const struct 
> ovs_key_ipv6 *ipv6_key)
> -   if (memcmp(ipv6_key->ipv6_dst, daddr, sizeof(ipv6_key->ipv6_dst))) {
> +   mask_ipv6_addr(saddr, key->ipv6_src, mask->ipv6_src, masked);
> +
> +   if (unlikely(memcmp(saddr, masked, sizeof masked))) {
> +   set_ipv6_addr(skb, key->ipv6_proto, saddr, masked,
> + true);

set_ipv6_addr() does a memcpy as well, I think that's now unnecessary
given that we are masking into the packet directly.

> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index 69d1919..ec32a00 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -1682,12 +1743,45 @@ static int validate_set(const struct nlattr *a,
> +   /* Convert non-masked non-tunnel set actions to masked set actions. */
> +   if (!masked && key_type != OVS_KEY_ATTR_TUNNEL) {
> +   int start, len = key_len * 2;
> +   struct nlattr *at;
> +
> +   *skip_copy = true;
> +
> +   start = add_nested_action_start(sfa, 
> OVS_ACTION_ATTR_SET_MASKED);
> +   if (start < 0)
> +   return start;
> +
> +   at = __add_action(sfa, key_type, NULL, len);
> +   if (IS_ERR(at))
> +   return PTR_ERR(at);
> +
> +   memcpy(nla_data(at), nla_data(ovs_key), key_len); /* Key. */
> +   /* Even though all-ones mask includes non-writeable fields,
> +* which we do not allow above, we will not actually set them
> +* when we execute the masked set action. */
> +   memset(nla_data(at) + key_len, 0xff, key_len);/* Mask. */

What about IPv6 flow label? I think we need to actually unmask those
bits since we want to retain the original value in the packet instead
of zeroing them.

> +   } else if (masked) {
> +   /* Reject fully masked masked set actions so that the above
> +* conversion can be unabiguously reverted when sending
> +* actions back to userspace. */
> +   if (is_all_ones(nla_data(ovs_key) + key_len, key_len))
> +   return -EINVAL;

It kind of pains me to have these types of exceptions from the
perspective of the new protocol that we are introducing. Is there any
way to achieve this effect without introducing a user-visible
interface quirk?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] route-table: Remove Unregister.

2014-11-25 Thread YAMAMOTO Takashi
> Since dpif registering for routing table at initialization
> there is no need to unregister it. Following patch removes
> support for turning routing table notifications on and off.
> Due to this change OVS always listens for these
> notifications.

probably rename route_table_register to route_table_init or such?

> diff --git a/lib/route-table-bsd.c b/lib/route-table-bsd.c
> index 9ebfaa3..0d631b4 100644
> --- a/lib/route-table-bsd.c
> +++ b/lib/route-table-bsd.c
> @@ -118,18 +118,7 @@ route_table_get_change_seq(void)
>  void
>  route_table_register(void)
>  {
> -if (!register_count)
> -{
> -pid = getpid();
> -}
> -
> -register_count++;
> -}
> -
> -void
> -route_table_unregister(void)
> -{
> -register_count--;
> +pid = getpid();
>  }
>  

please remove the register_count variable.

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


Re: [ovs-dev] [PATCH v5 12/13] datapath: Relax flow set-up time validations.

2014-11-25 Thread Jesse Gross
On Fri, Sep 5, 2014 at 4:05 PM, Jarno Rajahalme  wrote:
> Allow setting of fields without matching on the same fields.  Field
> existence check is done on set action execution time instead, using
> the extracted flow key.
>
> Signed-off-by: Jarno Rajahalme 

I guess it makes sense to switch to a runtime check at this point
given the direction that everything else is moving. However, you
mentioned doing this as part of make_writable() before, what is the
reason for looking at the flow key instead?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] hello

2014-11-25 Thread Favour
favour_richa...@yahoo.com
It is my great pleasure establishing a contact saw  your
email hoping we can be friends contact me with thislooking forward to hearing 
from you
Favour

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


[ovs-dev] hello

2014-11-25 Thread Naomi
naomiemman...@yahoo.com
It is my great pleasure establishing a contact with you when i saw your
email hoping we can be friends contact me with this
(naomiemman...@yahoo.com) looking forward to hearing from you
Naomi

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