On 8/8/14, 4:46 PM, "Andy Zhou" wrote:
>On Fri, Aug 8, 2014 at 3:58 PM, Pravin Shelar wrote:
>> On Fri, Aug 8, 2014 at 1:56 PM, Andy Zhou wrote:
>>> I agree with Daniele's comments.
>>>
>>> Acked-by: Andy Zhou
>>>
>> Thanks I pushed patch to master.
>>
>>> mask_set_nlattr() seems to have a s
This commit adds four options for configuring the MAC addresses
in BFD state machine. Therein, the "bfd_local_src_mac" and
"bfd_local_dst_mac" configure the MAC address of sent BFD
packets. The "bfd_remote_src_mac" and "bfd_remote_dst_mac"
configure the matching of MAC address on recevied BFD pac
This commit flips the default value of bfd ip source and destination,
so that they match the default value of ip destination and source
of vtep schema.
Signed-off-by: Alex Wang
---
lib/bfd.c|4 ++--
vswitchd/vswitch.xml |4 ++--
2 files changed, 4 insertions(+), 4 deletions(-
On Fri, Aug 8, 2014 at 3:58 PM, Pravin Shelar wrote:
> On Fri, Aug 8, 2014 at 1:56 PM, Andy Zhou wrote:
>> I agree with Daniele's comments.
>>
>> Acked-by: Andy Zhou
>>
> Thanks I pushed patch to master.
>
>> mask_set_nlattr() seems to have a single user. may be we cna remove
>> this function an
On Fri, Aug 8, 2014 at 1:56 PM, Andy Zhou wrote:
> I agree with Daniele's comments.
>
> Acked-by: Andy Zhou
>
Thanks I pushed patch to master.
> mask_set_nlattr() seems to have a single user. may be we cna remove
> this function and
> call set_nlattr() directly?
>
> Not part of this patch, but s
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. For the majority of ipv6 packets this would not be the case,
> so this can be marked as 'likely
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 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -688,6 +688,11 @@ static int ovs_key_from_nlattrs(struct sw_flow_match
> *ma
On Fri, Aug 8, 2014 at 4:47 AM, YAMAMOTO Takashi wrote:
>> commit (datapath: Refactor action alloc and copy api) effectively
>> reverted 1d2a1b5f5252e4c6ce8bbf8d91ca27aba52496e6 (datapath: Factor out
>> allocation and verification of actions.). This results in following
>> warning:
>>
>> CC [M] /
I agree with Daniele's comments.
Acked-by: Andy Zhou
mask_set_nlattr() seems to have a single user. may be we cna remove
this function and
call set_nlattr() directly?
Not part of this patch, but set_nlattr() does not look correct in
handling nested attributes.
On Fri, Aug 8, 2014 at 9:25 AM,
On Fri, Aug 8, 2014 at 1:28 PM, Jarno Rajahalme wrote:
> 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:
>> One difficulty that comes to mind is that for tunnel operations we
>> j
I pushed it to master.
Thanks.
On Fri, Aug 8, 2014 at 12:41 PM, Andy Zhou wrote:
> The rest of the patch also looks good. Thanks for making the changes.
>
> Acked-by: Andy Zhou
>
> On Thu, Aug 7, 2014 at 4:32 PM, Jarno Rajahalme wrote:
>> I’ll let Andy review the recirculation part in the bott
When I wrote we handle cancelation of pending IRPs" I meant that we provide the
I/O manager with the driver IRP cancel routine. Since theer are always pending
IRPs in the driver we must provide this mechanism.
Here is the MSDN description:
" Any driver in which IRPs can be held in a pending stat
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/actions.c |
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,
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/
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 --g
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
Add a new action type OVS_ACTION_ATTR_SET_MASKED, and support for
parsing, printing, and committing them.
Masked set actions add a mask, immediately following the netlink
attribute data, within the netlink attribute itself. Thus the key
attribute size for a masked set action is exactly double of
This version translates all non-masked set actions to masked set
actions in the Linux kernel module, apart from the set tunnel action,
for which masking makes no sense. This greatly simplifies the
changes, avoiding duplication of code comared to the previous version.
The userspace parts have been
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
>>> wrote:
Masked set ac
No problem Sam. It must be late there :-) Thank you for looking into the
details.
Eitan
-Original Message-
From: Samuel Ghinet [mailto:sghi...@cloudbasesolutions.com]
Sent: Friday, August 08, 2014 1:14 PM
To: Eitan Eliahu; Alin Serdean; dev@openvswitch.org; Rajiv Krishnamurthy; Ben
Pf
Oh, sorry. In my test cases, I was always completing the IRP requests
immediately. In my test cases, I was expecting Cancel IRPs to happen after
FilterUnload, but instead I was still receiving IRP writes & reads.
Given that in your code you also return status = pending, the setting of cancel
rou
>Are you sure the Cancel Irp callback that you set actually does get called?
The process could call
[QUOTE]We don't want to add per FILE_OBJECT buffer management.
[/QUOTE]
How would you do different for dumps?
I mean, during the time you issue a DeviceIoControl / read for a dump, if
another th
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/ofproto.c
> index 6bcec1
[QUOTE]We handle process termination and we cancel the pending IRPs
[/QUOTE]
Regarding process termination, I told you you don't need to do that, because a
Close IRP is coming when the process crashes / file handle is closed.
Also, regarding pending IRPs, I don't remember anywhere in your code (th
The rest of the patch also looks good. Thanks for making the changes.
Acked-by: Andy Zhou
On Thu, Aug 7, 2014 at 4:32 PM, Jarno Rajahalme wrote:
> 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
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 arbitrarily maskable or in 'match' otherwise.
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.3 says that
> non-exp
Hi,
I think we are all on same page now (following jesse,ben and samuel's
conversation).
There can be different approaches to solve this issue (none of them
significantly better than other).
I am going ahead with what ben recommended, i.e generating odp-netlink for
windows in datapath-windows
The email address of every contributor is in AUTHORS. (We occasionally
forget to add one, please submit a patch to add yourself if you're in
that boat.)
On Fri, Aug 08, 2014 at 06:17:00PM +, Samuel Ghinet wrote:
> I don't have his email address.
> Should I attempt a PATCH?
>
> Sam
>
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. The code her
We handle process termination and we cancel the pending IRPs. We don't want to
add per FILE_OBJECT buffer management.
Regarding the flow dump, I am not opposing to use your method to compute the
amount of memory as long as that the read process will be in parts )rather
allocating a huge buffer)
[QUOTE]This means that the driver does not have to track per process resources
so if the process is killed or crashed nothing needs to be done.
[/QUOTE]
[QUOTE]To maintain per process and per transaction resources or buffers
requires a lot of book keeping as well as handling abnormal termination
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
> farther down,
On Fri, Aug 8, 2014 at 11:09 AM, Ben Pfaff wrote:
> On Fri, Aug 08, 2014 at 06:03:38PM +, Samuel Ghinet wrote:
>> [QUOTE]If the transformation scripts become hard to maintain, we'll do
>> something else.[/QUOTE]
>> The script method still doesn't sound awesome.
>>
>> let's see... can we enclos
> Basically, this means that you don't need to provide an "offset" from
> userspace for a DeviceIoControl, unless you want the kernel to restrain
> himself from providing the whole dump reply at once.
This may work for DP and Port dump (provided the buffer is big enough. But for
flow the dump is
Sam,
We want to avoid maintaining state for a dump or a transaction in the driver.
This means that the driver does not have to track per process resources so if
the process is killed or crashed nothing needs to be done. I believe it is a
simpler approach. It means also that user mode should keep
I don't have his email address.
Should I attempt a PATCH?
Sam
From: Ben Pfaff [b...@nicira.com]
Sent: Friday, August 08, 2014 9:09 PM
To: Samuel Ghinet
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] odp-netlink.h: Autogenerate a version of
odp-netl
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Another word for "The only "state" we use in kernel for dumps is the buffer
itself, with its offset. It is not a "dump" offset, but a buffer offset.":
I remember the linux ovs kernel code, that it used some kind of "dump offset"
(or, I don't know how to call it, that "index-based" dumping).
I did
Eitan,
[QUOTE]).For example the offset for a port dump would be the last port which
was returned by the previous nl_dump_next().[/QUOTE]
Our kernel implementation of the netlink component does not maintain such state
for dumps.
The only "state" we use in kernel for dumps is the buffer itself, wi
On Fri, Aug 08, 2014 at 06:03:38PM +, Samuel Ghinet wrote:
> [QUOTE]If the transformation scripts become hard to maintain, we'll do
> something else.[/QUOTE]
> The script method still doesn't sound awesome.
>
> let's see... can we enclose the #include-s of openvswitch.h within an
> "#ifndef _
[QUOTE]If the transformation scripts become hard to maintain, we'll do
something else.[/QUOTE]
The script method still doesn't sound awesome.
let's see... can we enclose the #include-s of openvswitch.h within an "#ifndef
_WIN32"?
This way, we could #include openvswitch.h on windows.
And, if we ne
Hello guys,
I wanted to ask you about this since a week or so.
I have seen that you use the OvsIpHelper to find dest eth for a given target ip
(to be used for destination hypervisor).
I find the OvsIpHelper functionality quite complicated, and with calls to it in
quite some places within the pr
On Fri, Aug 08, 2014 at 05:16:11PM +, Samuel Ghinet wrote:
> [QUOTE]I posted patches for such a solution a few months back. Jesse rejected
> this approach because it caused differences between openvswitch.h
> in-tree and in upstream Linux.
> [/QUOTE]
> The alternative would be to simply use a
Sam, we want to avoid any state maintenance in the driver. The "offset" here
is not a buffer offset rather an "offset" related to the specific dump command
(DP. Port or Flow). For example the offset for a port dump would be the last
port which was returned by the previous nl_dump_next().
The I
Hello Eitan,
[QUOTE]> You cannot do this dump() operation in a single DeviceIoControl
request.
Certainly, not. I meant that each nl_dump_next use DeviceIOControl to retrieve
the next chunk of data. We need to pass down to the driver an offset which
indicates where this current chunk of data sta
Thanks,
Daniele
On 8/8/14, 10:20 AM, "Jarno Rajahalme" wrote:
>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 Raja
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 Proietto
>> wrote:
>>
[QUOTE]I posted patches for such a solution a few months back. Jesse rejected
this approach because it caused differences between openvswitch.h
in-tree and in upstream Linux.
[/QUOTE]
The alternative would be to simply use a separate "openvswitch.h" for windows.
This will add the issue of "duplica
Sam,
Let's take the whole event mechanism (as currently implemented) out of this
context as it is not Netlink specific, I will try to find your original email
about Events.
> You cannot do this dump() operation in a single DeviceIoControl request.
Certainly, not. I meant that each nl_dump_next u
Hello Eitan,
This discussion is getting more and more intrincate, with quotes of quotes of
quotes :)
[QUOTE]Currently there are two IOCTLs implanted in the driver. One which just
read an event from the event queue and is called synchronously. The other one
which is used just for the purpose of
On Fri, Aug 08, 2014 at 02:40:52PM +, Samuel Ghinet wrote:
> I've got an idea, tell me what you think:
> We could replace "openvswitch.h" on both platforms with something more
> generic and platform independent.
> This would hold, say, the enum constants and msg constants. The
> platform-inde
Hi Pravin,
couple of comments:
- I would remove also the check for match->mask in update_range__().
- I think braces are not necessary for if/else statements in the macros.
Of course I would wait for review by Jesse or Andy, but other than that,
LGTM.
Acked-by: Daniele Di Proietto
On 8/7/14,
Hi Sam,
Please find inline.
>Do you mean we will no longer use nl_sock_transact_multiple in userspace for
>these DPIF transactions?
No, we will use nl_sock_transact_multiple, but nl_sock_transact_multiple will
be implemented through the call of DeviceIOControl() system call rather than se
seri
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
> --- a/lib/flow.c
> +++ b/lib/flo
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
> message
> without blocking th
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 5e04015..2e5ca0a 100644
> --- a/lib/flow.
The reasons I see we'd need to support both devices would be to keep the rest
functionality working while we replace a part with its netlink counterpart.
[QUOTE]The two-device approach is for us to make sure that we are getting
things correctly. Eg. what is the output of flow dump in one device,
Hello Eitan,
[QUOTE]Transaction based DPIF primitives are mapped into synchronous device
I/O control system calls.
The NL reply would be returned in the output buffer of the IOCTL
parameter.[/QUOTE]
I am still confused. You spoke in the design file about
"nl_sock_transact_multiple", which coul
Hello guys,
I've got an idea, tell me what you think:
We could replace "openvswitch.h" on both platforms with something more generic
and platform independent.
This would hold, say, the enum constants and msg constants. The
platform-independent "openvswitch.h" must not #include any header.
Then,
Hello guys!
Thanks for your feedback!
I will make a patch with the file renaming then!
I am totally for keeping the CamelCase, both in code, and in file names!
Saurabh, regarding this:
[QUOTE]The code can freely call platform specific routines that don't adhere
to the convention (for e.g. Win
Hello,
Thanks for the feedback!
I'll try to stick better to the issues on github :)
Justin,
regarding referencing isuses in github, like "ovs/ovs-issues#42", thanks for
the idea!
I understand that this must be put in the text associated with the patch, and
this would allow linking once it's
acc
hello Nithin,
[QUOTE]If your idea is to post a patch that we could potentially use[/QUOTE]
Yeah, that was the idea. I was thinking that this attribute mapping may not
conflict with the possible new architecture. And I was thinking that a possible
issue on this would have been replied back.
But,
Hello Alin,
These are mere suggestions I have, and I am eager to hear any feedback on them.
I am certainly not trying to impose on the community to use a specific coding
style rule!
I am very curious on any coding style I have suggested that other people may
agree upon.
Thanks!
Sam
___
Hello Saurabh,
Well, these are functionalities that would have been used within the port
mapping files.
However, given the fact that we should postpone the port mapping integration, I
think it's best to postpone the List.h as well.
Sam
From: Saurabh Shah
Hello Saurabh,
I think you're right. I should have had a bit of patience :)
This code is mostly taken from our working ovs, so it should be ok - but you're
right, it cannot be tested yet, and should be postponed for the future.
Also, the netlink part dealing with vports and packet upcall is a bit
Hi Saurabh,
What I particularly don't like about these:
a) "atomic_add64" is not that much shorter than "InterlockedAdd64". When used
in code, I don't think you need to do much casting (to be actually worth using
the casting from within "atomic_add64")
b) Interlocked functions may not be called
Hello back,
Thanks for your reviews on this idea of mine!
What I had also had in mind, that I came up with this idea, was something that
I had found very confusing:
ATM we've got functions of km-um communication in flow, vport, etc. We also got
hyper-v switch port functions in vports. We've got
This patch implements the vhost-net offload API. It adds support for
a new port type to userspace datapath called dpdkvhost. This allows KVM
(QEMU) to offload the servicing of virtio-net devices to it's associated
dpdkvhost port. Instructions for use are in INSTALL.DPDK.
This has been tested on I
Hi All,
I want to incorporate "Eviction mechanism" as per the openflow
specification 1.4
As per the openflow specification 1.4 (Section 7.3.4.1), there are three
eviction flags: importance,lifetime and other. If importance flag is the
only flag set (lifetime and others not set), eviction is pe
> commit (datapath: Refactor action alloc and copy api) effectively
> reverted 1d2a1b5f5252e4c6ce8bbf8d91ca27aba52496e6 (datapath: Factor out
> allocation and verification of actions.). This results in following
> warning:
>
> CC [M] /home/jesse/openvswitch/datapath/linux/datapath.o
> /home/jesse
> On Thu, Jul 24, 2014 at 12:53:30PM -0700, Ben Pfaff wrote:
>> The umask is a process-wide value, so bind_unix_socket() races with file
>> creation in other Open vSwitch threads. This fixes the race.
>>
>> The workaround for non-Linux systems is not ideal, but I do not know any
>> other general
Extend IPFIX exporter to export tunnel headers when both input and output
of the port.
Add three other_config options in IPFIX table: enable-input-sampling,
enable-output-sampling and enable-tunnel-sampling, to control whether
sampling tunnel info, on which direction (input or output).
Insert sampl
Sorry, please ingore it. There is a mistake in the patch, I will resend the
patch soon.
-Original Message-
From: Wenyu Zhang
Sent: Friday, August 08, 2014 6:12 PM
To: Wenyu Zhang; Romain Lenglet; pshe...@nicira.com; b...@nicira.com; Jesse
Gross; dev@openvswitch.org
Subject: [PATCH v8] E
Extend IPFIX exporter to export tunnel headers when both input and output
of the port.
Add three other_config options in IPFIX table: enable-input-sampling,
enable-output-sampling and enable-tunnel-sampling, to control whether
sampling tunnel info, on which direction (input or output).
Insert sampl
4ävª_RÈ(A/ïFÆÄéX?â½±
×"* ;q¾edÍ#q,'lyUâTcÕj¶&êÐ5 ׳ùÁÉ>ð-÷TYܳ]¤Q´hb:¬ËÍÊn
lÎÕÌwØê¨?æHÌ{õj[Eµl'Üx;æ´%'Øduá¢`·Qd¡·´¿.øç½¥~ ú1Æ"ñ^ëÕ ½çò·{ò<
êÀ49?á¶ä.¶¡¯Þr¿Ã»%$%½
íí#Þ0ßííå1åýâ!>dX[ ó#æ%~6>A±IÌzWãèåë°Ct8²õ
Æh§9ô|7j6ÕîHd|á{&ñíªÀÓá3þè°ÚôøhFFBö£åY'è#ÏJ§
77 matches
Mail list logo
