Also, I suggest:
o) multiline comments - to have one style for them only.
I personally prefer:
/*
* comment_line1
* comment_line2
* comment_line3
*/
Others may prefer:
/* comment_line1
* comment_line2
* comment_line3 */
We would need to decide, and keep it one way only.
o) function declaration
On 29 August 2014 15:57, Pravin Shelar wrote:
>
> I have not finished review but I was wonder if you can get rid of the
> attributes bit mask also?
>
Hi Pravin,
I took a look, and I think I can reduce it down to just being inside
match_validate() without much trouble. I don't have a good feel f
Ben,
The tabs are automatically transformed into spaces when the email is sent.
Could you please replace the tabs in automake.mk into spaces before applying
the patch?
Thanks,
Sam
From: Samuel Ghinet
Sent: Friday, August 29, 2014 7:06 AM
To: dev@openvswit
This patch includes the file renaming and accommodations needed for the file
renaming to build the forwarding extension for Hyper-V.
This patch is also a follow-up for the thread:
http://openvswitch.org/pipermail/dev/2014-August/044005.html
The original automake.mk file uses tabs instead of spac
I see that the automake.mk file in the repository has tabs instead of spaces.,
while the patch believes the original automake.mk has spaces.
I'll re-create the patch, to also transform the tabs of automake.mk into spaces.
Sam
From: Ben Pfaff [b...@nicira.
On Tue, Aug 26, 2014 at 4:50 PM, Joe Stringer wrote:
> Signed-off-by: Joe Stringer
> ---
> datapath/flow_netlink.c | 219
> +++
> 1 file changed, 86 insertions(+), 133 deletions(-)
>
> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
>
Hi Alin,
On the polling thing, I thought we will be using single, out of band ,
outstanding I/O. This I/O can be pended in recv_wait. When the outstanding I/O
gets completed we will poll_immediate_wake().Then, we don't have to poll in the
_recv function rather just reading synchronously packet
Hello Nithin,
> /* Netlink datapath family. */
> NETLINK_CMD nlDatapathFamilyCmdOps[] = {
> { OVS_DP_CMD_GET, OvsGetDpCmdHandler,
> OVS_WRITE_DEV_OP | OVS_READ_DEV_OP, FALSE }
> };
Could you make the NETLINK_CMD variable initialization in the same style as the
NETLINK_FAMILY variable i
Hello Nithin,
> typedef struct _OVS_USER_PARAMS_CONTEXT {
>PIRP irp;
>POVS_OPEN_INSTANCE ovsInstance;
>UINT32 devOp;
Perhaps it would be useful to add a comment for devOp, something like:
UINT32 devOp; /* a value of OVS_*_DEV_OP */
since it may not be obvious that devOp is a 'flags'
Hello Nithin,
>BOOLEAN validateDp; /* Does command require a valid DP argument. */
> } NETLINK_CMD, *PNETLINK_CMD;
Perhaps a better name would be validateDpIfIndex? Or, is there going to be
something more to be validated later on, when validateDp = TRUE?
>if (nlFamilyOp
Rename dpif_linux -> dpif_netlink the actual file can be renamed as well
in the future.
Bypass all epoll functionality.
Treat dpif_netlink_recv__ with lazy polling using GetOverlappedResult
with the newly added overlapped structure in netlink.c
Initialize dpif_netlink_class on MSVC as well and d
Hello Nithin,
I have a few notes regarding the patch:
> struct {
> POVS_MESSAGE ovsMsg;/* OVS message passed during dump start. */
> UINT32 index[8];/* markers to continue dump from. One or more
> * of them may be used. */
> } d
> Or it may be that InterlockedExchange64() already does that?
Doesn't look like alignment is checked:
LONGLONG val = 0;
InterlockedCompareExchange64(&val, 0, 0);
00013F811006 xor eax,eax
00013F811008 mov qword ptr [val],rcx
00013F81100D lock cm
> -Original Message-
> From: Gurucharan Shetty [mailto:shet...@nicira.com]
> Sent: Thursday, August 28, 2014 12:10 PM
> To: Saurabh Shah
> Cc: Eitan Eliahu; dev@openvswitch.org; Gurucharan Shetty
> Subject: Re: [ovs-dev] [PATCH 2/2] cccl: Enable compiler optimization by
> default.
>
> On
Describe the steps required to setup use of travis-ci for any
GitHub ovs repository.
Signed-off-by: Thomas Graf
---
INSTALL | 43 +++
1 file changed, 43 insertions(+)
diff --git a/INSTALL b/INSTALL
index 7e0097b..15e93c5 100644
--- a/INSTALL
+++ b/INSTALL
On Aug 28, 2014, at 9:57 AM, Gurucharan Shetty wrote:
> diff --git a/lib/ovs-atomic-msvc.h b/lib/ovs-atomic-msvc.h
> new file mode 100644
> index 000..f357545
> --- /dev/null
> +++ b/lib/ovs-atomic-msvc.h
> @@ -0,0 +1,370 @@
> +/*
> + * Copyright (c) 2014 Nicira, Inc.
> + *
> + * Licensed und
Hi NIthin,
Changes are fine.
One minor comment:
1. In function OvsGetDpCmdHandler, we should check for devOp at line 830 rather
than dump_state.
This comment can be addressed later, we are good for the checkin.
Acked-by: Ankur Sharma
Regards,
Ankur
Fr
Hi Nithin,
Changes are fine.
One minor comment:
Add a description for each member of OVS_USER_PARAMS_CONTEXT
This comment can be addressed later we are good for checkin.
Acked-by: Ankur Sharma
Regards,
Ankur
From: dev on behalf of Nithin Raju
Sent:
Hi Nithin,
Changes are fine.
Acked-by: Ankur Sharma
Regards,
Ankur
From: dev on behalf of Nithin Raju
Sent: Thursday, August 28, 2014 10:59 AM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH 2/4] datapath-windows: make NL version a UIN8 and
add
Hi NIthin,
Changes are fine.
One very minor comment would be that we should improve the formatting of
InitUserDumpState function. But this can be handled later, we are good for
checkin.
Acked-by: Ankur Sharma
Regards,
Ankur
From: dev on behalf of Nit
On Wed, Aug 27, 2014 at 4:13 AM, Andy Zhou wrote:
> For the out of tree OVS module, the network stack recursion limit are
> some times lower than the default value enforced by dev.c.
> example that the default
>
> This patch implements a lower limit, than the limit enforced by dev,c,
> to accommod
On Wed, Aug 27, 2014 at 4:13 AM, Andy Zhou wrote:
> Since kernel stack is limited in size, it is not wise to using
> recursive function with large stack frames.
>
> This patch provides an alternative implementation of recirc action
> without using recursion.
>
> A per CPU fixed sized, 'deferred ac
On Wed, Aug 27, 2014 at 4:13 AM, Andy Zhou wrote:
> Split ovs_dp_packet_flow_lookup() into its own API. In preparation for
> the next patch.
>
We can use existing ovs_dp_process_packet() for recurc case, so no
need to restructure code.
___
dev mailing li
On Wed, Aug 27, 2014 at 4:13 AM, Andy Zhou wrote:
> Future pathces will make use of those functions.
>
> Signed-off-by: Andy Zhou
> ---
> datapath/Modules.mk | 1 +
> datapath/actions.c | 57
> +
> datapath/actions.h | 54 ++
On Thu, Aug 28, 2014 at 01:31:04PM -0700, Gurucharan Shetty wrote:
> > I'd normally expect 64-bit reads and write to be atomic when we're
> > building for x86-64:
> >> +/* 64 bit reads and write are not atomic on x86.
> Currently, we are only doing 32 bit builds. So, I put the following in
> ovs-at
> I'd normally expect 64-bit reads and write to be atomic when we're
> building for x86-64:
>> +/* 64 bit reads and write are not atomic on x86.
Currently, we are only doing 32 bit builds. So, I put the following in
ovs-atomic.h to only include this file for 32 bit builds :
...
#elif _MSC_VER && de
On Thu, Aug 28, 2014 at 09:57:42AM -0700, Gurucharan Shetty wrote:
> Before this change (i.e., with pthread locks for atomics on Windows),
> the benchmark for cmap and hmap was as follows:
>
> $ ./tests/ovstest.exe test-cmap benchmark 1000 3 1
> Benchmarking with n=1000, 3 threads, 1.00% m
On Fri, Aug 22, 2014 at 01:58:29PM -0700, Jarno Rajahalme wrote:
> Neither 'miss_config', 'n_missed', nor 'n_matched' is used to
> synchronize the state of any other variable, so we can use relaxed
> atomic operations on them.
>
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
_
On Fri, Aug 22, 2014 at 01:58:28PM -0700, Jarno Rajahalme wrote:
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Fri, Aug 22, 2014 at 01:58:27PM -0700, Jarno Rajahalme wrote:
> Neither 'enable_megaflows', 'udpif->flow_limit', 'udpif->n_flows', nor
> 'udpif->n_flows_timestamp' are used to synchronize the state of any
> other variables, so we can use relaxed atomic operations to access
> them.
>
> Move the
On Fri, Aug 22, 2014 at 01:58:26PM -0700, Jarno Rajahalme wrote:
> 'netflow_count' and the existence of actual netflow objects is not
> tightly synchronized, so we can use the relaxed atomic_count for it.
>
> Signed-off-by: Jarno Rajahalme
...
> -/* Returns true if there exist any netflow objec
On Fri, Aug 22, 2014 at 01:58:25PM -0700, Jarno Rajahalme wrote:
> 'miimon_cnt' and the actual device miimon configuration is only
> loosely coupled, so we can use the relaxed atomic_count for it.
>
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
___
On Fri, Aug 22, 2014 at 01:58:24PM -0700, Jarno Rajahalme wrote:
> Even though there is no need to optimize netdev-dummy, it might be
> good to do this right, in case it serves as an inspiration for
> something else later.
>
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
_
On Thu, Aug 28, 2014 at 12:09 PM, Gurucharan Shetty wrote:
> On Thu, Aug 28, 2014 at 11:45 AM, Saurabh Shah wrote:
>>> -Original Message-
>>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Gurucharan
>>> Shetty
>>> Sent: Thursday, August 28, 2014 10:54 AM
>>> To: Eitan Eliahu
On Fri, Aug 22, 2014 at 01:58:23PM -0700, Jarno Rajahalme wrote:
> All access to struct netdev_registered_class ref_cnt member was done
> with netdev_class_mutex held, so it does not need to be an atomic
> variable.
>
> Signed-off-by: Jarno Rajahalme
Good catch. Thank you.
_
On Thu, Aug 28, 2014 at 11:45 AM, Saurabh Shah wrote:
>> -Original Message-
>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Gurucharan
>> Shetty
>> Sent: Thursday, August 28, 2014 10:54 AM
>> To: Eitan Eliahu
>> Cc: dev@openvswitch.org; Gurucharan Shetty
>> Subject: Re: [ovs
> -Original Message-
> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Gurucharan
> Shetty
> Sent: Thursday, August 28, 2014 10:54 AM
> To: Eitan Eliahu
> Cc: dev@openvswitch.org; Gurucharan Shetty
> Subject: Re: [ovs-dev] [PATCH 2/2] cccl: Enable compiler optimization by
> defa
That would be very helpful. Thanks!
Saurabh
>Hi,
>
>Will send the patch until the end of the day.
>
>Alin.
>
>-Mesaj original-
>De la: Saurabh Shah [mailto:ssaur...@vmware.com]
>Trimis: Thursday, August 28, 2014 9:31 PM
>Către: Alin Serdean; dev@openvswitch.org; Ben Pfaff
>Subiect: Re:
Hi,
Will send the patch until the end of the day.
Alin.
-Mesaj original-
De la: Saurabh Shah [mailto:ssaur...@vmware.com]
Trimis: Thursday, August 28, 2014 9:31 PM
Către: Alin Serdean; dev@openvswitch.org; Ben Pfaff
Subiect: Re: [ovs-dev] [PATCH 3/4] Changes needed to compile dpif-linu
On Fri, Aug 22, 2014 at 01:58:22PM -0700, Jarno Rajahalme wrote:
> 'dump->status' does not syncronize the state of any other variable, so
> we can use relaxed atomics on it.
>
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
___
dev mailing list
d
Hi Alin,
We are getting to a point where we need this change. Do you plan to send a
V2 soon?
Thanks!
Saurabh
>Hi Saurabh,
>
>If Ben is ok with the name change I would be glad to send out a V2 of the
>patch.
>
>Thanks,
>Alin.
>
>-Mesaj original-
>De la: Saurabh Shah [mailto:ssaur...@vmwa
OVS tunnel compat code depends on this function pointer to
handle GSO packet. Currently we do not initialize for all
GRE GSO packets. Following patch fixes that.
Signed-off-by: Pravin B Shelar
---
datapath/linux/compat/gre.c |2 ++
datapath/linux/compat/gso.h |4 ++--
2 files changed, 4
On Thu, Aug 28, 2014 at 11:28:41AM -0700, Ben Pfaff wrote:
> On Fri, Aug 22, 2014 at 01:58:21PM -0700, Jarno Rajahalme wrote:
> > The atomics here do not synchronize the state of any other variables,
> > so we can use relaxed atomics.
> >
> > cfm_should_process_flow() is rearranged to set the mega
On Fri, Aug 22, 2014 at 01:58:21PM -0700, Jarno Rajahalme wrote:
> The atomics here do not synchronize the state of any other variables,
> so we can use relaxed atomics.
>
> cfm_should_process_flow() is rearranged to set the megaflow mask bits
> only if necessary, and to avoid the atomic operation
I didn't realize earlier that version in a netlink message was a
UINT8. So, fixing that here.
Also, some of the commands don't pass a valid DP value. Hence adding
a field to identify such commands.
Signed-off-by: Nithin Raju
---
datapath-windows/ovsext/Datapath.c | 74 +++-
In this patch we add a context structure for collecting all the parameters
passed from usersapce in one place. The idea is to reduce the number of
parameters being passed to the netlink command handler functions.
It can be argued that not all functions require all the arguments, but this
approach
In this patch, we add support for the GET_DP netlink command to dump
the datpaaths. The userspace workflow to get this to work is the same
as on Linux. dpif-linux.c initiates a dump start by writing a netlink
message, and after that continues to read data from the kernel while
the kernel has data.
Signed-off-by: Nithin Raju
---
datapath-windows/ovsext/Datapath.h | 54 ++--
1 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.h
b/datapath-windows/ovsext/Datapath.h
index 6d8a6db..0b303a2 100644
--- a/datapath-wind
On Thu, Aug 28, 2014 at 10:45 AM, Eitan Eliahu wrote:
>
> Hi Guru,
> Do we distinguish between Debug and Releases builds? ( I recall we discussed
> this before but forgot what was the outcome)
> Enabling optimization may cause some issues for the debugger to match the
> source to the code.
We cu
Before this change (i.e., with pthread locks for atomics on Windows),
the benchmark for cmap and hmap was as follows:
$ ./tests/ovstest.exe test-cmap benchmark 1000 3 1
Benchmarking with n=1000, 3 threads, 1.00% mutations:
cmap insert: 61070 ms
cmap iterate: 2750 ms
cmap search: 14238 m
Hi Guru,
Do we distinguish between Debug and Releases builds? ( I recall we discussed
this before but forgot what was the outcome)
Enabling optimization may cause some issues for the debugger to match the
source to the code.
Thanks,
Eitan
-Original Message-
From: dev [mailto:dev-boun..
There is no 1-1 mapping between gcc's compiler optimization
flags and MSVC's optimization flags making the automatic
conversions trickier.
This commit adds the /O2 option which is the recommended
option for fast code.
Signed-off-by: Gurucharan Shetty
---
build-aux/cccl |2 +-
1 file changed
The /FS option allows serial access to PDB file creation letting
parallel builds succeed with mingw32-make (with some tricks). The
'make' that comes with MSYS has a bug that causes hangs with
parallel builds which supposedly has been fixed in the upcoming
1.0.19 release.
Signed-off-by: Gurucharan
On Fri, Aug 22, 2014 at 01:58:20PM -0700, Jarno Rajahalme wrote:
> The atomics here do not synchronize the state of any other variables,
> so we can use atomic_count and relaxed atomics.
>
> bfd_should_process_flow() is rearranged to set the megaflow mask bits
> only if necessary, and to avoid the
On Fri, Aug 22, 2014 at 01:58:19PM -0700, Jarno Rajahalme wrote:
> Trivial ID counters do not synchronize anything, therefore can use
> atomic_count.
>
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http:
On Fri, Aug 22, 2014 at 01:58:18PM -0700, Jarno Rajahalme wrote:
> Avoiding the atomic read may help if a function using
> ovsthread_once_start() is ever called in a loop, as the new
> 'maybe_not_done' can be kept in a register. The atomic read will
> still be done as long as 'maybe_not_done' is t
On Fri, Aug 22, 2014 at 01:58:17PM -0700, Jarno Rajahalme wrote:
> barrier->count is used as a simple counter and is not expected the
> synchronize the state of any other variable, so we can use atomic_count,
> which uses relaxed atomics.
>
> Ditto for the 'next_id' within ovsthread_wrapper().
>
On Fri, Aug 22, 2014 at 01:58:16PM -0700, Jarno Rajahalme wrote:
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Fri, Aug 22, 2014 at 01:58:15PM -0700, Jarno Rajahalme wrote:
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
On Fri, Aug 22, 2014 at 01:58:14PM -0700, Jarno Rajahalme wrote:
> When an atomic variable is not serving to synchronize threads about
> the state of other (atomic or non-atomic) variables, no memory barrier
> is needed with the atomic operation. However, the default memory
> order for an atomic o
On Fri, Aug 22, 2014 at 01:58:13PM -0700, Jarno Rajahalme wrote:
> ovs_refcount_unref() needs to syncronize with the other instances of
> itself rather than with ovs_refcount_ref().
>
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
___
dev mailin
I didn't know that, thanks!
On Thu, Aug 28, 2014 at 03:54:26PM +, Eitan Eliahu wrote:
> If you run it from a bash you must add the .exe extension:
>
> Administrator@hyperv-target2 /c/kern
> $ msbuild
> sh: msbuild: command not found
>
> Administrator@hyperv-target2 /c/kern
> $ msbuild.exe
>
On Fri, Aug 22, 2014 at 01:58:12PM -0700, Jarno Rajahalme wrote:
> Otherwise the dereference operator could target a portion of a ternary
> expression, for example.
>
> Also minor style fixes.
>
> Signed-off-by: Jarno Rajahalme
Acked-by: Ben Pfaff
__
On Wed, Aug 27, 2014 at 08:36:19AM -0700, Nithin Raju wrote:
> The Windows datapath supports a READ/WRITE ioctl instead of
> ReadFile/WriteFile.
> In this change, we update the following:
> - WriteFile() in nl_sock_send__() to use DeviceIoControl(OVS_IOCTL_WRITE)
> - ReadFile() in nl_sock_recv__()
If you run it from a bash you must add the .exe extension:
Administrator@hyperv-target2 /c/kern
$ msbuild
sh: msbuild: command not found
Administrator@hyperv-target2 /c/kern
$ msbuild.exe
Microsoft (R) Build Engine version 12.0.21005.1
[Microsoft .NET Framework, version 4.0.30319.33440]
Copyright
Applied, thanks!
On Thu, Aug 28, 2014 at 06:05:40AM -0700, Jarno Rajahalme wrote:
> Acked-by: Jarno Rajahalme
>
>
> > On Aug 27, 2014, at 9:31 AM, Ben Pfaff wrote:
> >
> > C99 only requires compilers to support four types for bit-fields: signed
> > int, unsigned int, int, and _Bool. "int" sh
On Thu, Aug 28, 2014 at 01:49:24PM +, Alin Serdean wrote:
> +if VSTUDIO_DDK
> +ALL_LOCAL += ovsext_make
> +ovsext_make: datapath-windows/ovsext.sln
> + MSBuild.exe datapath-windows/ovsext.sln /target:Build
> /property:Configuration="$(VSTUDIO_CONFIG)"
Isn't it unusual to include the .exe
On Thu, Aug 28, 2014 at 03:37:23PM +, Saurabh Shah wrote:
>
> >This commit adds to the automake build system the full build required
> >by the forwarding extension solution.
> >
> >It will help a lot in the future CI to check the full build of the
> >project.
> >
> >To configure the forwarding
On Thu, Aug 28, 2014 at 02:40:50PM +0200, Thomas Graf wrote:
> Although the check for negative timeout is present, the error string
> is overwritten if an invalid "until" is found right after. This leaks
> an error string and results in not reporting the negative timeout back
> to the user even tho
>This commit adds to the automake build system the full build required
>by the forwarding extension solution.
>
>It will help a lot in the future CI to check the full build of the
>project.
>
>To configure the forwarding extension to be built one could use the
>following:
>./configure CC=./build-a
Acked-by: Alin Gabriel Serdean
Kind Regards,
Alin.
From: dev [dev-boun...@openvswitch.org] on behalf of Eitan Eliahu
[elia...@vmware.com]
Sent: Thursday, August 28, 2014 1:01 AM
To: Eitan Eliahu; Nithin Raju
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev]
This commit adds to the automake build system the full build required
by the forwarding extension solution.
It will help a lot in the future CI to check the full build of the project.
To configure the forwarding extension to be built one could use the following:
./configure CC=./build-aux/cccl L
Acked-by: Jarno Rajahalme
> On Aug 27, 2014, at 9:31 AM, Ben Pfaff wrote:
>
> C99 only requires compilers to support four types for bit-fields: signed
> int, unsigned int, int, and _Bool. "int" should not be used because it
> is implementation-defined whether it is signed. In practice, we ha
Although the check for negative timeout is present, the error string
is overwritten if an invalid "until" is found right after. This leaks
an error string and results in not reporting the negative timeout back
to the user even though it is encountered first.
Signed-off-by: Thomas Graf
---
v2: Bet
74 matches
Mail list logo
