[ovs-dev] [PATCH] lib/route-table-*: Fix non-Linux builds
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
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)
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
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
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
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
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
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.
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.
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.
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
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
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.
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.
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
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
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.
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
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.
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
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
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
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
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
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.
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.
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
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
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
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
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
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
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
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.
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.
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.
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.
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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()
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.
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().
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.
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.
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.
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+
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.
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.
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.
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.
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.
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.
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.
=== 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.
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.
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.
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().
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.
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
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
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
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.
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.
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.
> 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
> 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.
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
> 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
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.
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.
> 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.
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
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
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