Re: [ovs-dev] [ext-244 1/4] meta-flow: Simplify handling of a variable number of registers.

2014-07-26 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Jul 25, 2014, at 10:25 PM, Ben Pfaff wrote: > At the time that Open vSwitch implemented registers, there was a high cost > to adding additional fields, so I wrote the code so that the number of > registers could be reduced at compile time. Now, fields ar

Re: [ovs-dev] [ext-244 2/4] bitmap: Add new functions.

2014-07-26 Thread Jarno Rajahalme
LGTM, Acked-by: Jarno Rajahalme On Jul 25, 2014, at 10:25 PM, Ben Pfaff wrote: > These will be used in an upcoming commit. > > Signed-off-by: Ben Pfaff > --- > lib/bitmap.c | 44 > lib/bitmap.h | 6 ++ > 2 files c

Re: [ovs-dev] [ext-244 3/4] Remove assumption that there are 64 or fewer fields.

2014-07-26 Thread Jarno Rajahalme
Two minor questions for clarification below, Jarno Acked-by: Jarno Rajahalme On Jul 25, 2014, at 10:25 PM, Ben Pfaff wrote: > An upcoming commit will increase the number of fields beyond 64. > > Signed-off-by: Ben Pfaff > --- > lib/classifier.c | 9 +++-- > lib

Re: [ovs-dev] [ext-244 4/4] meta-flow: Add 64-bit registers.

2014-07-26 Thread Jarno Rajahalme
Some questions for clarification below, Jarno Acked-by: Jarno Rajahalme On Jul 25, 2014, at 10:25 PM, Ben Pfaff wrote: > These 64-bit registers are intended to conform with the OpenFlow 1.5 > draft specification. > > EXT-244. > Signed-off-by: Ben Pfaff > --- > incl

Re: [ovs-dev] [ext-244 3/4] Remove assumption that there are 64 or fewer fields.

2014-07-26 Thread Jarno Rajahalme
Sent from my iPhone > On Jul 26, 2014, at 10:37 AM, Ben Pfaff wrote: > > On Sat, Jul 26, 2014 at 08:47:56AM -0700, Jarno Rajahalme wrote: >>> +/* A set of mf_field_ids. */ >>> +struct mf_bitmap { >>> +unsigned long bm[BITMAP_N_LONGS(MFF_N_IDS)]; >&g

Re: [ovs-dev] [ext-244 3/4] Remove assumption that there are 64 or fewer fields.

2014-07-26 Thread Jarno Rajahalme
On Jul 26, 2014, at 10:41 AM, Jarno Rajahalme wrote: > > > Sent from my iPhone > >> On Jul 26, 2014, at 10:37 AM, Ben Pfaff wrote: >> >> On Sat, Jul 26, 2014 at 08:47:56AM -0700, Jarno Rajahalme wrote: >>>> +/* A set of mf_field_ids. */ >>&

Re: [ovs-dev] [ext-244 3/4] Remove assumption that there are 64 or fewer fields.

2014-07-26 Thread Jarno Rajahalme
On Jul 26, 2014, at 10:58 AM, Ben Pfaff wrote: > On Sat, Jul 26, 2014 at 10:41:34AM -0700, Jarno Rajahalme wrote: >>> On Jul 26, 2014, at 10:37 AM, Ben Pfaff wrote: >>> On Sat, Jul 26, 2014 at 08:47:56AM -0700, Jarno Rajahalme wrote: >>>>> -enum ofput

Re: [ovs-dev] [PATCH] datapath: Use currect rcu API in exact match flow lookup function.

2014-07-26 Thread Jarno Rajahalme
> On Jul 26, 2014, at 5:53 PM, Pravin B Shelar wrote: > > exact match cache lookup is always done under ovs lock. So > use ovsl_dereference() API for rcu access. > This description makes it sound like exact match cache lookup would always be done under ovs lock, which just cannot be true. Do

[ovs-dev] [PATCH 1/6] lib/ovs-atomic: Add atomic_uint64_t and atomic_int64_t.

2014-07-27 Thread Jarno Rajahalme
Add non-standard atomic types atomic_uint64_t and atomic_int64_t. Use of these types is more natural in OVS code than the standard equivalents. Signed-off-by: Jarno Rajahalme --- lib/ovs-atomic.h |4 1 file changed, 4 insertions(+) diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h

[ovs-dev] [PATCH 3/6] lib/ovs-atomic: Require memory_order be constant.

2014-07-27 Thread Jarno Rajahalme
to comply with this. Signed-off-by: Jarno Rajahalme --- lib/cmap.c | 18 +++--- lib/ovs-atomic.h |3 +++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/cmap.c b/lib/cmap.c index ba744cc..60629b1 100644 --- a/lib/cmap.c +++ b/lib/cmap.c @@ -163,19

[ovs-dev] [PATCH 6/6] lib/ovs-atomic: Native support for x86_64 with GCC.

2014-07-27 Thread Jarno Rajahalme
port provided by this patch: Benchmarking with n=200, 8 threads, 0.10% mutations: cmap insert:530 ms cmap iterate:59 ms cmap search:305 ms cmap destroy: 232 ms Signed-off-by: Jarno Rajahalme --- lib/automake.mk |1 + lib/ovs-atomic-x8

[ovs-dev] [PATCH 5/6] tests/test-atomic: Cover more of the atomic API.

2014-07-27 Thread Jarno Rajahalme
This adds tests using all of the defined memory orders, as well as simple two-thread test cases for the acquire-release and consume-release patterns. These new tests helped uncover a bug in the ovs-atomic-gcc4+ implementation, which was fixed in a preceding patch. Signed-off-by: Jarno Rajahalme

[ovs-dev] [PATCH 0/6] Native atomics support for x86_64 on GCC.

2014-07-27 Thread Jarno Rajahalme
Our supported XenServer build environments lack proper atomic support, which makes recent OVS techiques (ovs-rcu, cmap) painfully slow. This series enhances OVS atomic support to address this issue. Corresponding 32-bit support is still to be done. Jarno Rajahalme (6): lib/ovs-atomic: Add

[ovs-dev] [PATCH 4/6] lib/ovs-atomic-gcc4+: Use 'volatile' to enforce memory access.

2014-07-27 Thread Jarno Rajahalme
thout this change the more rigorous atomic test cases introduced in a following patch will hang due to the atomic accesses being optimized away. Signed-off-by: Jarno Rajahalme --- lib/ovs-atomic-gcc4+.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ovs-atomic-gcc

[ovs-dev] [PATCH 2/6] lib/ovs-atomic: Elaborate memory_order documentation.

2014-07-27 Thread Jarno Rajahalme
The definition of memory_order_relaxed included a compiler barrier, while it is not necessary, and indeed the following text on atomic_thread_fence and atomic_signal_fence contradicted that. memory_model_consume is also more thoroughly described. Signed-off-by: Jarno Rajahalme --- lib/ovs

Re: [ovs-dev] [PATCH 1/6] lib/ovs-atomic: Add atomic_uint64_t and atomic_int64_t.

2014-07-27 Thread Jarno Rajahalme
> On Jul 27, 2014, at 1:58 PM, Ben Pfaff wrote: > >> On Sun, Jul 27, 2014 at 10:45:40AM -0700, Jarno Rajahalme wrote: >> Add non-standard atomic types atomic_uint64_t and atomic_int64_t. Use >> of these types is more natural in OVS code than the standard >> equ

Re: [ovs-dev] [PATCH] cmap: Merge CMAP_FOR_EACH_SAFE into CMAP_FOR_EACH.

2014-07-28 Thread Jarno Rajahalme
Ben, Thanks for reminding, somehow I lost track of this. This is actually a nice cleanup :-) Acked-by: Jarno Rajahalme On Jul 21, 2014, at 9:38 PM, Ben Pfaff wrote: > There isn't any significant downside to making cmap iteration "safe" all > the time, so this dr

[ovs-dev] [PATCH v2 1/7] Fix strict aliasing violations with GCC 4.1 and 4.4.

2014-07-31 Thread Jarno Rajahalme
lattr. After this patch there are no further warnings with the XenServer build, so we could start treating warnings as errors in the builds. Signed-off-by: Jarno Rajahalme --- ofproto/connmgr.c | 12 ofproto/ofproto-dpif-sflow.c | 12 +++- ofproto/ofproto-dpif-upc

[ovs-dev] [PATCH v2 0/7] Native atomics support for i586 and x86_64 with GCC.

2014-07-31 Thread Jarno Rajahalme
on newer versions. Jarno Rajahalme (7): Fix strict aliasing violations with GCC 4.1 and 4.4. lib/ovs-atomic: Elaborate memory_order documentation. lib/ovs-atomic: Require memory_order be constant. lib/ovs-atomic-gcc4+: Use 'volatile' to enforce memory access. tests/test-atomic:

[ovs-dev] [PATCH v2 2/7] lib/ovs-atomic: Elaborate memory_order documentation.

2014-07-31 Thread Jarno Rajahalme
The definition of memory_order_relaxed included a compiler barrier, while it is not necessary, and indeed the following text on atomic_thread_fence and atomic_signal_fence contradicted that. memory_model_consume is also more thoroughly described. Signed-off-by: Jarno Rajahalme --- lib/ovs

[ovs-dev] [PATCH v2 5/7] tests/test-atomic: Cover more of the atomic API.

2014-07-31 Thread Jarno Rajahalme
This adds tests using all of the defined memory orders, as well as simple two-thread test cases for the acquire-release and consume-release patterns. These new tests helped uncover a bug in the ovs-atomic-gcc4+ implementation, which was fixed in a preceding patch. Signed-off-by: Jarno Rajahalme

[ovs-dev] [PATCH v2 4/7] lib/ovs-atomic-gcc4+: Use 'volatile' to enforce memory access.

2014-07-31 Thread Jarno Rajahalme
s use of a volatile cast mirrors the Linux kernel ACCESS_ONCE macro. Without this change the more rigorous atomic test cases introduced in a following patch will hang due to the atomic accesses being optimized away. Signed-off-by: Jarno Rajahalme --- lib/ovs-atomic-gcc4+.h |4 ++-- 1 file

[ovs-dev] [PATCH v2 3/7] lib/ovs-atomic: Require memory_order be constant.

2014-07-31 Thread Jarno Rajahalme
, ovs-rcu, and cmap to comply with this. Signed-off-by: Jarno Rajahalme --- v2: Also update ovs-rcu accordingly. lib/cmap.c | 18 +-- lib/ovs-atomic-gcc4+.h | 10 - lib/ovs-atomic.h |3 +++ lib/ovs-rcu.h | 57

[ovs-dev] [PATCH v2 6/7] lib/ovs-atomic: Native support for x86_64 with GCC.

2014-07-31 Thread Jarno Rajahalme
by this patch: Benchmarking with n=200, 8 threads, 0.10% mutations: cmap insert:530 ms cmap iterate:59 ms cmap search:305 ms cmap destroy: 232 ms Signed-off-by: Jarno Rajahalme --- v2: Use macros to avoid repetitive asm blocks. lib/automake.mk |1 + lib/ovs-atomic-x8

[ovs-dev] [PATCH v2 7/7] lib/ovs-atomic: Native support for 32-bit 586 with GCC.

2014-07-31 Thread Jarno Rajahalme
:68 ms cmap search:353 ms cmap destroy: 209 ms Signed-off-by: Jarno Rajahalme --- lib/automake.mk |1 + lib/ovs-atomic-i586.h | 452 + lib/ovs-atomic.h |2 + 3 files changed, 455 insertions(+) create mode 100644 lib/ovs

Re: [ovs-dev] [PATCH v2 0/7] Native atomics support for i586 and x86_64 with GCC.

2014-07-31 Thread Jarno Rajahalme
I’m posting a V3 soon, so if anyone is planning to review, wait a bit more… Jarno On Jul 31, 2014, at 9:17 AM, Jarno Rajahalme wrote: > Our supported XenServer build environments lack proper atomic > libraries, which makes recent OVS techiques (ovs-rcu, cmap) painfully > slow. Th

[ovs-dev] [PATCH v3 0/8] Native atomics support for i586 and x86_64 with GCC.

2014-07-31 Thread Jarno Rajahalme
, but such compilations are not for performance in the first place. - Fixed atomic_flag to default to memory_order_seq_cst for GCC4+, and both new implementations. Jarno Rajahalme (8): Fix strict aliasing violations with GCC 4.1 and 4.4. lib/ovs-atomic: Elaborate memory_order documentation.

[ovs-dev] [PATCH v3 1/8] Fix strict aliasing violations with GCC 4.1 and 4.4.

2014-07-31 Thread Jarno Rajahalme
lattr. After this patch there are no further warnings with the XenServer build, so we could start treating warnings as errors in the builds. Signed-off-by: Jarno Rajahalme --- ofproto/connmgr.c | 12 ofproto/ofproto-dpif-sflow.c | 12 +++- ofproto/ofproto-dpif-upc

[ovs-dev] [PATCH v3 3/8] lib/ovs-atomic: Require memory_order be constant.

2014-07-31 Thread Jarno Rajahalme
, however, that when inlining is disabled (i.e., compiling without optimization) even compile-time constants may be passed as run-time values to (non-inlined) functions. Signed-off-by: Jarno Rajahalme --- lib/ovs-atomic-gcc4+.h | 10 -- lib/ovs-atomic.h |4 2 files changed

[ovs-dev] [PATCH v3 6/8] tests/test-atomic: Cover more of the atomic API.

2014-07-31 Thread Jarno Rajahalme
This adds tests using all of the defined memory orders, as well as simple two-thread test cases for the acquire-release and consume-release patterns. These new tests helped uncover a bug in the ovs-atomic-gcc4+ implementation, which was fixed in a preceding patch. Signed-off-by: Jarno Rajahalme

[ovs-dev] [PATCH v3 7/8] lib/ovs-atomic: Native support for x86_64 with GCC.

2014-07-31 Thread Jarno Rajahalme
by this patch: Benchmarking with n=200, 8 threads, 0.10% mutations: cmap insert:530 ms cmap iterate:59 ms cmap search:305 ms cmap destroy: 232 ms Signed-off-by: Jarno Rajahalme --- lib/automake.mk |1 + lib/ovs-atomic-x8

[ovs-dev] [PATCH v3 4/8] ovs-atomic: Fix GCC4+ atomic_flag.

2014-07-31 Thread Jarno Rajahalme
The default memory order for atomic_flag is documented to be memory_order_seq_cst (as in C11), but the GCC4+ implementation only used the GCC builtins, which provide acquire and release semantics only. Additional barriers are needed for in other cases. Signed-off-by: Jarno Rajahalme --- lib

[ovs-dev] [PATCH v3 5/8] lib/ovs-atomic-gcc4+: Use 'volatile' to enforce memory access.

2014-07-31 Thread Jarno Rajahalme
s use of a volatile cast mirrors the Linux kernel ACCESS_ONCE macro. Without this change the more rigorous atomic test cases introduced in a following patch will hang due to the atomic accesses being optimized away. Signed-off-by: Jarno Rajahalme --- lib/ovs-atomic-gcc4+.h |4 ++-- 1 file

[ovs-dev] [PATCH v3 2/8] lib/ovs-atomic: Elaborate memory_order documentation.

2014-07-31 Thread Jarno Rajahalme
The definition of memory_order_relaxed included a compiler barrier, while it is not necessary, and indeed the following text on atomic_thread_fence and atomic_signal_fence contradicted that. memory_order_consume and memory_order_acq_rel are also more thoroughly described. Signed-off-by: Jarno

[ovs-dev] [PATCH v3 8/8] lib/ovs-atomic: Native support for 32-bit 586 with GCC.

2014-07-31 Thread Jarno Rajahalme
:68 ms cmap search:353 ms cmap destroy: 209 ms Signed-off-by: Jarno Rajahalme --- lib/automake.mk |1 + lib/ovs-atomic-i586.h | 440 + lib/ovs-atomic.h |2 + 3 files changed, 443 insertions(+) create mode 100644 lib/ovs

Re: [ovs-dev] [PATCH v3 7/8] lib/ovs-atomic: Native support for x86_64 with GCC.

2014-08-01 Thread Jarno Rajahalme
Figured this out last night: On Jul 31, 2014, at 3:21 PM, Jarno Rajahalme wrote: (snip) > +#define atomic_compare_exchange__(DST, EXP, SRC, RES, CLOB) \ > +asm volatile("lock; cmpxchg %3,%1 ; " \ > +

Re: [ovs-dev] [PATCH v3 7/8] lib/ovs-atomic: Native support for x86_64 with GCC.

2014-08-05 Thread Jarno Rajahalme
Thanks for the reviews! I have pushed the series to master, some answers/comments below, Jarno On Aug 5, 2014, at 11:35 AM, Ben Pfaff wrote: > > Your research is far stronger than mine on this. I have only a few > comments. > > This ignores the possibility of misaligned atomic variables > (

Re: [ovs-dev] Help with error in when compiling custom library

2014-08-05 Thread Jarno Rajahalme
It might be helpful starting with the exact compile command line used by “make” on lib/my_library.c, it will have all the right include directory options you are missing from your gcc command line that might have an effect. Given that your successful gcc line misses all the “-I” directives, I su

[ovs-dev] [PATCH 3/6] ofproto: Check actions also for packet outs and traces.

2014-08-05 Thread Jarno Rajahalme
Make the packet out and trace processing perform the same actions checks as flow mod processing does. This used to be the case before, but at some point these have diverged to perform different combinations of checks. Signed-off-by: Jarno Rajahalme --- ofproto/ofproto-dpif.c | 30

[ovs-dev] [PATCH 2/6] ovs-ofctl: Fix a typo in documentation.

2014-08-05 Thread Jarno Rajahalme
Signed-off-by: Jarno Rajahalme --- utilities/ovs-ofctl.8.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 5251c53..77eddb4 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1986,7 +1986,7

[ovs-dev] [PATCH 4/6] dpif: Meter framework.

2014-08-05 Thread Jarno Rajahalme
Add DPIF-level infrastructure for meters. Allow meter_set to modify the meter configuration (e.g. set the burst size if unspecified). Signed-off-by: Jarno Rajahalme --- datapath/linux/compat/include/linux/openvswitch.h |3 + lib/dpif-linux.c | 40

[ovs-dev] [PATCH 6/6] dpif-netdev: Simple DROP meter implementation.

2014-08-05 Thread Jarno Rajahalme
meter bands are hit, we need to process the packets individually. Signed-off-by: Jarno Rajahalme --- lib/dpif-netdev.c| 362 +++--- tests/dpif-netdev.at | 105 +++ 2 files changed, 450 insertions(+), 17 deletions(-) diff --git a/lib

[ovs-dev] [PATCH 1/6] utilities/ovs-ofctl: Fix meter requests.

2014-08-05 Thread Jarno Rajahalme
Meter requests should use dump/stats transaction, instead of transact_noreply, which caused the output to go to stderr and an error exit. Signed-off-by: Jarno Rajahalme --- utilities/ovs-ofctl.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utilities/ovs-ofctl.c b

[ovs-dev] [PATCH 5/6] lib/odp, ofproto xlate: Meter execution support.

2014-08-05 Thread Jarno Rajahalme
leaks less likely to appear as new kinds of action are added later. Signed-off-by: Jarno Rajahalme --- lib/dpif-netdev.c| 31 ++--- lib/dpif.c | 44 +++-- lib/netdev-bsd.c |5 lib/netdev-dpdk.c

Re: [ovs-dev] datapath-windows: Could we rename the files please?

2014-08-05 Thread Jarno Rajahalme
I for one like the fact that it is possible to find almost any (*) symbol that matters day-to-day in OVS source code with a simple "grep foo */*.[ch]". I'm not working on the Windows port, but this is an argument for a current flat directory structure. Jarno (*) include directory and Linux c

Re: [ovs-dev] [PATCH 4/6] dpif: Meter framework.

2014-08-06 Thread Jarno Rajahalme
> On Aug 5, 2014, at 6:19 PM, Jesse Gross wrote: > >> On Tue, Aug 5, 2014 at 4:38 PM, Jarno Rajahalme >> wrote: >> diff --git a/datapath/linux/compat/include/linux/openvswitch.h >> b/datapath/linux/compat/include/linux/openvswitch.h >> index 271a14e..64

Re: [ovs-dev] datapath-windows: Could we rename the files please?

2014-08-06 Thread Jarno Rajahalme
> On Aug 6, 2014, at 2:48 AM, Samuel Ghinet > wrote: > > Hello Nithin, > > I'd like to highlight two points here: > 1. the "datapath" on linux doesn't have all files Ovs-prefixed. I don't think > there's a specific requirement for > datapath-windows for that. > 2. the "datapath" on linux uses

Re: [ovs-dev] [PATCH] datapath: Optimize Flow mask cache hash collision case.

2014-08-06 Thread Jarno Rajahalme
One suggestion below, otherwise looks right to me, Acked-by: Jarno Rajahalme On Aug 5, 2014, at 3:25 PM, Pravin B Shelar wrote: > In case hash collision on mask cache, OVS does extra flow lookup. > Following patch avoid it. > > Suggested-by: Jarno Rajahalme > Signed-off-by:

Re: [ovs-dev] [PATCH] test-atomic: Fix warnings for GCC4.9 and sparse

2014-08-06 Thread Jarno Rajahalme
Daniele, Thanks for spotting these. GCC error message may be cryptic, but using an atomic variable as the return value is an error regardless. Acked-by: Jarno Rajahalme Pushed to master, Jarno On Aug 6, 2014, at 10:35 AM, Daniele Di Proietto wrote: > There's no reason for t

Re: [ovs-dev] [PATCH 2/2] flow: Provide a better explanation why L4 state is in L3 section.

2014-08-06 Thread Jarno Rajahalme
> On Aug 6, 2014, at 2:55 PM, Justin Pettit wrote: > > A future commit will add more L4 state fields that will be part of the > L3 section. > > Signed-off-by: Justin Pettit > --- > lib/flow.h |5 - > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/lib/flow.h b/lib/flo

Re: [ovs-dev] [PATCH] datapath: Optimize recirc action.

2014-08-07 Thread Jarno Rajahalme
Most of the time we do not need the updated key (i.e., when there is no recirc action). Do you think it would be worthwhile to only update the key when needed, e.g. by adding a “bool update_key;” member to struct sw_flow_actions? More comments below, Jarno On Aug 7, 2014, at 12:32 PM, Pravin

Re: [ovs-dev] [PATCH] datapath: Refactor action alloc and copy api.

2014-08-07 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 12:51 PM, Pravin B Shelar wrote: > There are two separate API to allocate and copy actions list. Anytime > OVS needs to copy action list, it needs to call both functions. > Following patch moves action allocation to copy function to avo

Re: [ovs-dev] [PATCH 1/9] ofp-util: Remove prototypes for unimplemented functions.

2014-08-07 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 4, 2014, at 9:21 AM, Ben Pfaff wrote: > Signed-off-by: Ben Pfaff > --- > lib/ofp-util.h | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/lib/ofp-util.h b/lib/ofp-util.h > index 23396bb..39e50ed 100644 > --- a/lib/ofp-util.

Re: [ovs-dev] [PATCH 2/9] ofp-actions: Add action bitmap abstraction.

2014-08-07 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme Three minor comments below, Jarno On Aug 4, 2014, at 9:21 AM, Ben Pfaff wrote: > Until now, sets of actions have been abstracted separately outside > ofp-actions, as enum ofputil_action_bitmap. Drawing sets of actions into > ofp-actions, as done in th

Re: [ovs-dev] [PATCH 3/9] ofp-actions: Add instructions bitmaps and fix related bug.

2014-08-07 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme Two small nits below, Jarno On Aug 4, 2014, at 9:21 AM, Ben Pfaff wrote: > This will allow, later, to centralize all of the knowledge of instruction > encoding inside ofp-actions. > > OFPIT11_ALL and OFPIT13_ALL are no longer used, so this commit re

Re: [ovs-dev] [PATCH 4/9] ofproto: Correctly report table miss configuration in table stats.

2014-08-07 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 4, 2014, at 9:21 AM, Ben Pfaff wrote: > OFPTC11_TABLE_MISS_MASK isn't a valid value at all, let alone always the > value in use. We should report the value actually in use, as this commit > does. > > There is a remaining proble

Re: [ovs-dev] [PATCH v2] datapath: Optimize recirc action.

2014-08-07 Thread Jarno Rajahalme
I’ll let Andy review the recirculation part in the bottom. For set actions: Acked-by: Jarno Rajahalme On Aug 7, 2014, at 3:46 PM, Pravin B Shelar wrote: > OVS need to flow key for flow lookup in recic action. OVS > does key extract in recic action. Most of cases we could > use OVS_

Re: [ovs-dev] [PATCH 1/3] lib/flow: Update FLOW_WC_SEQ to skip assertions on miniflow_extract()

2014-08-08 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme > On Aug 7, 2014, at 6:15 PM, Daniele Di Proietto > wrote: > > Signed-off-by: Daniele Di Proietto > --- > lib/flow.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/flow.c b/lib/flow.c > index 5e040

Re: [ovs-dev] [PATCH 2/3] Add BUILD_MESSAGE() macro

2014-08-08 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme Sent from my iPhone > On Aug 7, 2014, at 6:15 PM, Daniele Di Proietto > wrote: > > This commit introduces the BUILD_MESSAGE() macro. It uses _Pragma("message"), > with compilers that support that, to output a warning-like compile-time >

Re: [ovs-dev] [PATCH 3/3] lib/flow: Use BUILD_MESSAGE() to warn if BUILD_SEQ is not updated

2014-08-08 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme > On Aug 7, 2014, at 6:15 PM, Daniele Di Proietto > wrote: > > Signed-off-by: Daniele Di Proietto > --- > lib/flow.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/flow.c b/lib/flow.c > index 2e5ca0a..78b132e 100644

Re: [ovs-dev] [PATCH 1/3] lib/flow: Update FLOW_WC_SEQ to skip assertions on miniflow_extract()

2014-08-08 Thread Jarno Rajahalme
Pushed to master. No backports needed. As discussed offline, I’ll wait for Ben to chime in on the other two patches before pushing them. Jarno On Aug 8, 2014, at 9:09 AM, Jarno Rajahalme wrote: > Acked-by: Jarno Rajahalme > >> On Aug 7, 2014, at 6:15 PM, Daniele Di Proiett

Re: [ovs-dev] [PATCH v2 05/16] ofp-util: Abstract table miss configuration and fix related bugs.

2014-08-08 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme Two small nits and a comment error (?) spotted below, Jarno On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > The ofproto implementation has had an abstraction layer on top of > OFPTC11_TABLE_MISS for a while. This commit pushes that abstraction layer > far

Re: [ovs-dev] [PATCH v2 06/16] ofp-util: Fix table features decoding of multiple tables.

2014-08-08 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme So we do not have a test case that would pull table features for multiple tables from a single OpenFlow message? Jarno On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > Table features replies can be packed back-to-back within a single > multipart reply. Th

Re: [ovs-dev] [PATCH v2 07/16] ofp-util: Fix table features decoding of action and instruction fields.

2014-08-08 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > Parsing of actions was wrong because OF1.3 says that non-experimenter > actions are 4 bytes and don't include padding. This fixes the problem. > > Parsing of instructions seems wrong too because OF1.

Re: [ovs-dev] [PATCH v2 08/16] ofp-util: Fix table features decoding of match and mask.

2014-08-08 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > The call to parse_oxms() inside ofputil_decode_table_features() sets only > one bit in either 'match' or 'mask' for a given field that is matchable: > in 'mask' if the field is arbit

Re: [ovs-dev] [PATCH v2 09/16] ofproto: Implement OpenFlow 1.3+ table features request.

2014-08-08 Thread Jarno Rajahalme
Small nits below. Also, I did not verify this against the OpenFlow spec. Otherwise: Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > Signed-off-by: Ben Pfaff > — (snip) > struct ofputil_meter_band { > diff --git a/ofproto/ofproto.c b/ofproto/ofpro

Re: [ovs-dev] [PATCH v2 8/8] datapath: Allow masks for set actions.

2014-08-08 Thread Jarno Rajahalme
I’ve just posted a v3 of this series, Jarno On Aug 5, 2014, at 10:57 AM, Jesse Gross wrote: > On Fri, Jul 18, 2014 at 8:15 AM, Jarno Rajahalme > wrote: >> On May 8, 2014, at 1:27 PM, Jesse Gross wrote: >> >>> On Fri, Apr 11, 2014 at 4:29 PM, Jarno Rajahalme

[ovs-dev] [PATCH v3 0/6] Masked set actions

2014-08-08 Thread Jarno Rajahalme
been rebased, otherwise they are the same as before. Jarno Rajahalme (6): lib/odp: Masked set action execution and printing. ofproto: Probe for masked set action support. lib/odp: Use masked set actions. datapath/actions: Mark recalculate_csum as likely in set_ipv6_addr(). datapath

[ovs-dev] [PATCH v3 1/6] lib/odp: Masked set action execution and printing.

2014-08-08 Thread Jarno Rajahalme
the non-masked set action. Signed-off-by: Jarno Rajahalme --- datapath/linux/compat/include/linux/openvswitch.h |4 + lib/dpif-netdev.c |1 + lib/dpif.c|1 + lib/odp-execute.c

[ovs-dev] [PATCH v3 6/6] datapath: Allow masks for set actions.

2014-08-08 Thread Jarno Rajahalme
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. Signed-off-by: Jarno Rajahalme --- datapath/actions.c| 291

[ovs-dev] [PATCH v3 5/6] datapath/flow_netlink: Validate IPv6 flow key and mask values.

2014-08-08 Thread Jarno Rajahalme
Reject flow label key and mask values with invalid bits set. Signed-off-by: Jarno Rajahalme --- datapath/flow_netlink.c |5 + 1 file changed, 5 insertions(+) diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c index e4cf535..294e54c 100644 --- a/datapath/flow_netlink.c +++ b

[ovs-dev] [PATCH v3 4/6] datapath/actions: Mark recalculate_csum as likely in set_ipv6_addr().

2014-08-08 Thread Jarno Rajahalme
The ‘recalculate_csum’ is almost always ‘true’. It is false only if the ipv6 nexthdr is an extension header, and a routing header is found. For the majority of ipv6 packets this would not be the case, so this can be marked as 'likely'. Signed-off-by: Jarno Rajahalme --- datapath

[ovs-dev] [PATCH v3 3/6] lib/odp: Use masked set actions.

2014-08-08 Thread Jarno Rajahalme
Signed-off-by: Jarno Rajahalme --- lib/odp-util.c | 368 ++ lib/odp-util.h |5 +- ofproto/ofproto-dpif-xlate.c | 15 +- tests/ofproto-dpif.at| 62 --- tests/tunnel.at |8 +- 5 files changed

[ovs-dev] [PATCH v3 2/6] ofproto: Probe for masked set action support.

2014-08-08 Thread Jarno Rajahalme
Signed-off-by: Jarno Rajahalme Reviewed-by: YAMAMOTO Takashi --- ofproto/ofproto-dpif-xlate.c | 18 + ofproto/ofproto-dpif-xlate.h |3 ++- ofproto/ofproto-dpif.c | 58 +- 3 files changed, 72 insertions(+), 7 deletions(-) diff

Re: [ovs-dev] [PATCH v3 4/6] datapath/actions: Mark recalculate_csum as likely in set_ipv6_addr().

2014-08-11 Thread Jarno Rajahalme
Pushed, Jarno On Aug 8, 2014, at 2:12 PM, Jesse Gross wrote: > On Fri, Aug 8, 2014 at 1:28 PM, Jarno Rajahalme wrote: >> The ‘recalculate_csum’ is almost always ‘true’. It is false only if >> the ipv6 nexthdr is an extension header, and a routing header is >> found.

[ovs-dev] [PATCH v4 2/4] ofproto: Probe for masked set action support.

2014-08-11 Thread Jarno Rajahalme
Signed-off-by: Jarno Rajahalme Reviewed-by: YAMAMOTO Takashi --- ofproto/ofproto-dpif-xlate.c | 18 + ofproto/ofproto-dpif-xlate.h |3 ++- ofproto/ofproto-dpif.c | 58 +- 3 files changed, 72 insertions(+), 7 deletions(-) diff

[ovs-dev] [PATCH v4 0/4] Masked set actions.

2014-08-11 Thread Jarno Rajahalme
Rebased to the recent datapath changes in actions.c. Jarno Rajahalme (4): lib/odp: Masked set action execution and printing. ofproto: Probe for masked set action support. lib/odp: Use masked set actions. datapath: Allow masks for set actions. datapath/actions.c

[ovs-dev] [PATCH v4 1/4] lib/odp: Masked set action execution and printing.

2014-08-11 Thread Jarno Rajahalme
the non-masked set action. Signed-off-by: Jarno Rajahalme --- datapath/linux/compat/include/linux/openvswitch.h |4 + lib/dpif-netdev.c |1 + lib/dpif.c|1 + lib/odp-execute.c

[ovs-dev] [PATCH v4 3/4] lib/odp: Use masked set actions.

2014-08-11 Thread Jarno Rajahalme
Signed-off-by: Jarno Rajahalme --- lib/odp-util.c | 368 ++ lib/odp-util.h |5 +- ofproto/ofproto-dpif-xlate.c | 15 +- tests/ofproto-dpif.at| 62 --- tests/tunnel.at |8 +- 5 files changed

[ovs-dev] [PATCH v4 4/4] datapath: Allow masks for set actions.

2014-08-11 Thread Jarno Rajahalme
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. Signed-off-by: Jarno Rajahalme --- datapath/actions.c| 333

Re: [ovs-dev] [PATCH v3 5/6] datapath/flow_netlink: Validate IPv6 flow key and mask values.

2014-08-11 Thread Jarno Rajahalme
Thanks for the review, pushed to master with the proposed change. Jarno On Aug 8, 2014, at 2:10 PM, Jesse Gross wrote: > On Fri, Aug 8, 2014 at 1:28 PM, Jarno Rajahalme wrote: >> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c >> index e4cf535..294e54c

Re: [ovs-dev] [PATCH v2 10/16] ovs-ofctl: Un-document "apply_actions" because it does not exist.

2014-08-11 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > "apply_actions" is assumed, any other instruction has to be specified > explicitly. > > Signed-off-by: Ben Pfaff > --- > utilities/ovs-ofctl.8.in |3 --- > 1 file changed, 3 deletions

Re: [ovs-dev] [PATCH v2 11/16] ovs-ofctl: Fix error message in "parse-ofp10-actions" internal command.

2014-08-11 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > Signed-off-by: Ben Pfaff > --- > utilities/ovs-ofctl.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c > index 7b4a006..4

Re: [ovs-dev] [PATCH v2 12/16] ofp-parse: Make string conversion functions available outside this file.

2014-08-11 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > An upcoming commit will use them from ofp-actions. > > Signed-off-by: Ben Pfaff > --- > lib/ofp-parse.c | 14 +++--- > lib/ofp-parse.h | 13 - > 2 files changed, 19 inser

Re: [ovs-dev] [PATCH v2 13/16] ofp-actions: Pretend that OpenFlow 1.0 has instructions.

2014-08-11 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:13 PM, Ben Pfaff wrote: > This allows callers to be more uniform, because they don't have to pick > out whether they should parse actions or instructions based on the OpenFlow > version in use. It also allows the Write-Metada

Re: [ovs-dev] [PATCH v2 14/16] ofp-actions: Centralize all OpenFlow action code for maintainability.

2014-08-11 Thread Jarno Rajahalme
Nice cleanup, especially like the way wire formats are now hidden inside lib/ofp-actions.c. Some small comments below, Jarno Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:14 PM, Ben Pfaff wrote: > Until now, knowledge about OpenFlow has been somewhat scattered around the > tree.

Re: [ovs-dev] [PATCH v2 15/16] ovs-ofctl: Generalize action and instruction test commands.

2014-08-11 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:14 PM, Ben Pfaff wrote: > Until now, there were four of these commands: parse-ofp10-actions, > parse-ofp10-instructions, parse-ofp11-actions, parse-ofp11-instructions. > This is painful to add support for new OpenFlow versions and has

Re: [ovs-dev] [PATCH v2 16/16] ofp-actions: Add support for OpenFlow 1.5 (draft) Copy-Field action.

2014-08-11 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 7, 2014, at 4:14 PM, Ben Pfaff wrote: > ONF-JIRA: EXT-320 > Signed-off-by: Ben Pfaff > --- > NEWS |1 + > lib/ofp-actions.c| 80 ++ > tests/ofp-actions.at | 24 +++

Re: [ovs-dev] [PATCH 1/2] ofp-actions: Avoid logging 64 times as many actions as actually provided.

2014-08-11 Thread Jarno Rajahalme
Exponential bug :-) I find the identifier ‘max_actions’ still misleading, ‘actions_len’ would be better. Acked-by: Jarno Rajahalme On Aug 11, 2014, at 1:44 PM, Ben Pfaff wrote: > The following commit adds a test. > > Signed-off-by: Ben Pfaff > --- > lib/ofp-actions.c |

Re: [ovs-dev] [PATCH 2/2] ofp-actions: Use specific error code for oxm_hasmask=1 in Set-Field.

2014-08-11 Thread Jarno Rajahalme
Acked-by: Jarno Rajahalme On Aug 11, 2014, at 1:44 PM, Ben Pfaff wrote: > Based on the OpenFlow 1.5 draft. > > ONF-JIRA: EXT-425 > Signed-off-by: Ben Pfaff > --- > lib/ofp-actions.c|2 +- > lib/ofp-errors.h |4 > tests/ofp-actions.at | 12 ++

Re: [ovs-dev] [PATCH] datapath: Update flow key before recirc

2014-08-11 Thread Jarno Rajahalme
On Aug 11, 2014, at 2:22 PM, Andy Zhou wrote: > When flow key becomes invalid due to push or pop actions, current > implementation leaves it as invalid, only rebuild the flow key used > for recirculation. > > This works, but is less efficient in case of multiple recirc > actions. Each recirc ac

Re: [ovs-dev] [PATCH 5/6] lib/odp, ofproto xlate: Meter execution support.

2014-08-11 Thread Jarno Rajahalme
This needs a rebase due to the recent lib/ofp-actions.c changes. I’ll post a v2 in a moment. Jarno On Aug 5, 2014, at 4:38 PM, Jarno Rajahalme wrote: > Meter action can drop or modify packets, so the execution framework is > changed to allow for this. Also, a meter action can appear

[ovs-dev] [PATCH v2 2/6] ovs-ofctl: Fix a typo in documentation.

2014-08-11 Thread Jarno Rajahalme
Signed-off-by: Jarno Rajahalme --- utilities/ovs-ofctl.8.in |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index aafda23..b0a2c87 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1983,7 +1983,7

[ovs-dev] [PATCH v2 0/6] Meter implementation for userspace datapath.

2014-08-11 Thread Jarno Rajahalme
This is a rebase of the series due to recent changes in lib/ofp-actions.c. Jarno Rajahalme (6): utilities/ovs-ofctl: Fix meter requests. ovs-ofctl: Fix a typo in documentation. ofproto: Check actions also for packet outs and traces. dpif: Meter framework. lib/odp, ofproto xlate: Meter

[ovs-dev] [PATCH v2 4/6] dpif: Meter framework.

2014-08-11 Thread Jarno Rajahalme
Add DPIF-level infrastructure for meters. Allow meter_set to modify the meter configuration (e.g. set the burst size if unspecified). Signed-off-by: Jarno Rajahalme --- datapath/linux/compat/include/linux/openvswitch.h |3 + lib/dpif-linux.c | 40

[ovs-dev] [PATCH v2 3/6] ofproto: Check actions also for packet outs and traces.

2014-08-11 Thread Jarno Rajahalme
Make the packet out and trace processing perform the same actions checks as flow mod processing does. This used to be the case before, but at some point these have diverged to perform different combinations of checks. Signed-off-by: Jarno Rajahalme --- ofproto/ofproto-dpif.c | 30

[ovs-dev] [PATCH v2 5/6] lib/odp, ofproto xlate: Meter execution support.

2014-08-11 Thread Jarno Rajahalme
leaks less likely to appear as new kinds of action are added later. Signed-off-by: Jarno Rajahalme --- lib/dpif-netdev.c| 31 ++--- lib/dpif.c | 44 +++-- lib/netdev-bsd.c |5 lib/netdev-dpdk.c

[ovs-dev] [PATCH v2 6/6] dpif-netdev: Simple DROP meter implementation.

2014-08-11 Thread Jarno Rajahalme
meter bands are hit, we need to process the packets individually. Signed-off-by: Jarno Rajahalme --- lib/dpif-netdev.c| 362 +++--- tests/dpif-netdev.at | 105 +++ 2 files changed, 450 insertions(+), 17 deletions(-) diff --git a/lib

[ovs-dev] [PATCH v2 1/6] utilities/ovs-ofctl: Fix meter requests.

2014-08-11 Thread Jarno Rajahalme
Meter requests should use dump/stats transaction, instead of transact_noreply, which caused the output to go to stderr and an error exit. Signed-off-by: Jarno Rajahalme --- utilities/ovs-ofctl.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utilities/ovs-ofctl.c b

Re: [ovs-dev] [PATCH 2/3] Add BUILD_MESSAGE() macro

2014-08-12 Thread Jarno Rajahalme
Will do, tomorrow :-) Jarno > On Aug 12, 2014, at 1:58 PM, Ben Pfaff wrote: > > I'm happy enough with these patches. Jarno, do you want to apply > them? > >> On Fri, Aug 08, 2014 at 09:13:42AM -0700, Jarno Rajahalme wrote: >> Acked-by: Jarno Rajahalme >&

  1   2   3   4   5   6   7   8   9   10   >