I find this quite a bit simpler to reason about. Also, future patches
require xlate_actions() not to modify the packet.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-upcall.c | 25 ++
ofproto/ofproto-dpif-xlate.c | 80 +--
ofproto
In accordance with CodingStyle.
Signed-off-by: Ethan Jackson
---
lib/dpif-netdev.c | 4 ++--
lib/netdev-dpdk.c | 10 +-
ofproto/bundles.c | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index bbc4eea..76e36d9 100644
--- a
This patch causes classifier_lookup_miniflow_batch() to return a
boolean indicating whether any rules could not be successfully looked
up. Used in future patches.
Signed-off-by: Ethan Jackson
---
lib/classifier.c | 10 +++---
lib/classifier.h | 2 +-
2 files changed, 8 insertions(+), 4
Signed-off-by: Ethan Jackson
---
utilities/ovs-dev.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index 74143ca..496f6dc 100755
--- a/utilities/ovs-dev.py
+++ b/utilities/ovs-dev.py
@@ -131,6 +131,7 @@ def conf():
if clang
In future patches, dpif-netdev will not create a flow key for each
upcall. To accommodate this, we require callers to handle flow
parsing themselves.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-upcall.c | 30 --
ofproto/ofproto-dpif-xlate.c | 23
This patch gives dp_netdev_flow_add() a match with which it can
initialize the classifier rule. This prevents it from needing to copy
a flow and flow_wildcards into the match first.
Signed-off-by: Ethan Jackson
---
lib/dpif-netdev.c | 27 +++
1 file changed, 11
Necessary for future patches which need the provided flow to be const.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-upcall.c | 37 +++--
ofproto/ofproto-dpif-xlate.c | 23 ---
ofproto/ofproto-dpif-xlate.h | 6 +++---
ofproto
In future patches, this will allow dpif-netdev to avoid a copy.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-upcall.c | 16 -
ofproto/ofproto-dpif-xlate.c | 75 +++
ofproto/ofproto-dpif-xlate.h | 8 -
ofproto/ofproto-dpif.c
This problem is uncovered by a future patch.
Signed-off-by: Ethan Jackson
---
lib/flow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/flow.c b/lib/flow.c
index 5e04015..cfd90d6 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -210,7 +210,7 @@ parse_mpls(void **datap
This patch reorganizes miss handling so that it can support future
patches which don't go through the standard dpif interfaces. As an
added side benefit, the resulting code is a bit easier to understand.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-ipfix.c | 8 +-
ofproto/of
This patch avoids the relatively inefficient miss handling processes
dictated by the dpif process, by calling into ofproto-dpif directly
through a callback.
Signed-off-by: Ethan Jackson
---
lib/dpif-netdev.c | 299 --
lib/dpif-provider.h
1 through 10 are all ofproto reorganization, so I think you or jarno would be
appropriate reviewers.
Ethan (iPhone)
> On Aug 3, 2014, at 19:24, Ben Pfaff wrote:
>
>> On Fri, Aug 01, 2014 at 06:39:12PM -0700, Ethan Jackson wrote:
>> In accordance with CodingStyle.
>>
I did but I was lazy. I'll change it.
Ethan
On Tue, Aug 5, 2014 at 1:02 PM, Ben Pfaff wrote:
> On Fri, Aug 01, 2014 at 06:39:17PM -0700, Ethan Jackson wrote:
>> In future patches, this will allow dpif-netdev to avoid a copy.
>>
>> Signed-off-by: Ethan Jackson
>
&g
complexity within the xlate module.
Thoughts?
Ethan
On Tue, Aug 5, 2014 at 2:04 PM, Ethan Jackson wrote:
> I did but I was lazy. I'll change it.
>
> Ethan
>
> On Tue, Aug 5, 2014 at 1:02 PM, Ben Pfaff wrote:
>> On Fri, Aug 01, 2014 at 06:39:17PM -0700, Ethan Jackson wro
01, 2014 at 06:39:18PM -0700, Ethan Jackson wrote:
>> I find this quite a bit simpler to reason about. Also, future patches
>> require xlate_actions() not to modify the packet.
>>
>> Signed-off-by: Ethan Jackson
>
> This actually make xlate_actions() modify the packe
Based on my (long ago) reading of the LACP spec, only supporting a
single aggregator is a valid configuration. Furthermore, it's what
makes the most sense given the structure of the OVS bonding
configuration. I'd really rather not make a non standard change to
the protocol to support a buggy upst
This problem is uncovered by a future patch.
Signed-off-by: Ethan Jackson
---
lib/flow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/flow.c b/lib/flow.c
index 5e04015..64f7ba7 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -210,7 +210,7 @@ parse_mpls(void **datap
packet processing path should treat packets
and struct flow's as const.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-ipfix.c | 8 +-
ofproto/ofproto-dpif-ipfix.h | 4 +-
ofproto/ofproto-dpif-sflow.c | 2 +-
ofproto/ofproto-dpif-sflow.h | 6 +-
ofproto/ofproto-dpif-upc
This patch avoids the relatively inefficient miss handling processes
dictated by the dpif process, by calling into ofproto-dpif directly
through a callback.
Signed-off-by: Ethan Jackson
---
lib/dpif-netdev.c | 299 --
lib/dpif-provider.h
Acked-by: Ethan Jackson
On Fri, Aug 8, 2014 at 5:02 PM, Alex Wang wrote:
> 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
>
For the local eth_src and eth_dst, instead of having the various is
set booleans, wouldn't it be simpler to just copy the default value
into the struct bfd in bfd_configure() instead of doing it in
bfd_put_packet()?
I'm not sure we should have the rmt_eth_src flag. We should probably
accept bfd p
In OVS we only support a single aggregator per port. I understand
that Linux supports multiple, and that perhaps this problem would be
easier to work around if we did as well. But today we don't, and IMO
implementing this feature would be rather complex. Of course, an
implementation would be wel
> In ofproto-dpif-upcall.c, struct upcall seriously needs comments on
> the members. Some of them are baffling at first glance (put_actions,
> userdata, vsp_adjusted).
Done.
> I'm not sure of the value of the new 'put_actions' member. It only
> gets used in one place. That place can't compose_
> In ofproto-dpif-upcall.c, there are two blank lines above upcall_cb
> (horrors!).
I'm experimenting with a new form of avant-garde post-post-post-modern
dadaist syntax formatting. Only those with the most discerning taste
could possibly comprehend its momentous implications.
> Did you test wit
packet processing path should treat packets
and struct flow's as const.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-ipfix.c | 8 +-
ofproto/ofproto-dpif-ipfix.h | 4 +-
ofproto/ofproto-dpif-sflow.c | 2 +-
ofproto/ofproto-dpif-sflow.h | 6 +-
ofproto/ofproto-dpif-upc
This patch avoids the relatively inefficient miss handling processes
dictated by the dpif process, by calling into ofproto-dpif directly
through a callback.
Signed-off-by: Ethan Jackson
---
lib/dpif-netdev.c | 293 +++---
lib/dpif-provider.h
This problem is uncovered by a future patch.
Signed-off-by: Ethan Jackson
Acked-by: Ben Pfaff
---
lib/flow.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/flow.c b/lib/flow.c
index 2e5ca0a..29b331e 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -210,7 +210,7
Acked-by: Ethan Jackson
On Mon, Aug 11, 2014 at 6:34 PM, Alex Wang wrote:
> This commit adds 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
> p
Good catch. I've fixed it and will merge soon.
Ethan
On Thu, Aug 14, 2014 at 9:49 AM, Ben Pfaff wrote:
> On Wed, Aug 13, 2014 at 06:46:17PM -0700, Ethan Jackson wrote:
>> This patch reorganizes ofproto-dpif in preparation for future patches
>> which allow direct upcall
Due to a typo, the latest upcall refactoring caused dpif_recv() to be
called on an un-initialized chunk of memory.
Signed-off-by: Ethan Jackson
Reported-by: Justin Pettit
---
ofproto/ofproto-dpif-upcall.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif
Acked-by: Ethan Jackson
Thanks for taking care of this.
Ethan
On Fri, Aug 15, 2014 at 1:15 AM, Alex Wang wrote:
> Commit cc377352d (ofproto: Reorganize in preparation for direct
> dpdk upcalls.) introduced the bug that uses variable defined on
> the stack inside while loop for rea
warning: incorrect type in argument 1 (different base types)
expected restricted ovs_be16 [usertype] old_csum
got unsigned short [unsigned] [usertype] icmp6_cksum
Signed-off-by: Ethan Jackson
---
lib/packets.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/packets.c b/lib
oops, I missed it. I'll drop this then
On Tue, Sep 2, 2014 at 6:44 PM, Jesse Gross wrote:
> On Tue, Sep 2, 2014 at 6:36 PM, Ethan Jackson wrote:
>> warning: incorrect type in argument 1 (different base types)
>> expected restricted ovs_be16 [usertype] old_csum
>&
These options don't make sense when building portable code, but when
using the dev script, OVS is built on the same system it's run on.
They make a small difference in the OVS DPDK testing, hence their
addition.
Signed-off-by: Ethan Jackson
---
utilities/ovs-dev.py | 6 +++---
1 file
They may or may not make a difference, but there's no reason not to
support passing them.
Signed-off-by: Ethan Jackson
---
utilities/ovs-dev.py | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/utilities/ovs-dev.py b/utilities/ovs-dev.py
index e454e18..36
They have slightly different support characteristics, so it's nice to
easily switch between them for testing.
Signed-off-by: Ethan Jackson
---
utilities/ovs-dev.py | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/utilities/ovs-dev.py b/utilities/ovs-d
> Is there any point to testing OVS in a configuration that we won't
> distribute?
This is a subject in which there could legitimately be many points of
view, but I'll take a stab at explaining how I think about it.
To answer your question: In general no, but with DPDK yes.
If you look at how st
Acked-by: Ethan Jackson
On Thu, Sep 11, 2014 at 3:03 PM, Joe Stringer wrote:
> The datapath max_idle value determines how long to wait before deleting
> an idle datapath flow when operating below the flow_limit. This patch
> increases the max_idle to 10 seconds, which allows datapath
Acked-by: Ethan Jackson
On Sun, Sep 14, 2014 at 8:06 PM, Joe Stringer wrote:
> Although I reviewed and applied #1, I'll leave this one up to Ethan as it's
> more relevant to his other proposed changes.
>
> On 13 September 2014 05:35, Daniele Di Proietto
> wrote:
The patch introduces some trailing whitespace.
> - - If userspace's notion of the flow key for the packet matches the
> + - If the userspace's notion of the flow key for the packet matches the
I don't agree with this change. It's pretty common to talk about
userspace without a "the" as the REA
Typically adding yourself to the authors file wouldn't be a separate
patch, I'd just merge this into the previous one.
Also, Looking at the file now the first list of people are those who
have "authored or signed off on commits in the Open vSwitch source
code". I think the patch you suggested tec
> Also, there is a typo in the subject.
Doh, I should have caught that.
Ethan
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
put a v3 in the next patch.
>
> --Justin
>
>
>> On Jul 2, 2015, at 12:01 PM, Luis E Pena wrote:
>>
>> Should I redo this patch with my name in the first list?
>>
>>> On Jul 2, 2015, at 11:15, Ben Pfaff wrote:
>>>
>>> On Thu, Jul 02, 20
Acked-by: Ethan Jackson
Merged. Changed some tabs to spaces
Ethan
On Thu, Jul 2, 2015 at 12:34 PM, Luis E. P wrote:
> Signed-off-by: Luis E. P
> ---
> AUTHORS| 1 +
> datapath/README.md | 7 ---
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> d
similar systems. The hope is that it can be used as
a starting point for design discussions and an eventual
implementation.
Signed-off-by: Ethan Jackson
---
OVN-GW-HA.md | 374 +++
1 file changed, 374 insertions(+)
create mode 100644 OVN-GW
Thanks for the review, I've addressed the comments and will send out
another version of the patch shortly. One comment on the feedback
below.
> Under "Controller Independent Active-backup", I am not sure that I buy
> the argument here, because currently ovn-northd doesn't care about the
> layout
similar systems. The hope is that it can be used as
a starting point for design discussions and an eventual
implementation.
Signed-off-by: Ethan Jackson
---
ovn/OVN-GW-HA.md | 375 +++
1 file changed, 375 insertions(+)
create mode 100644 ovn/OVN-GW
No worries. I sent a new version (actually ran spell check on this one ;) )
On Tue, Jul 21, 2015 at 11:35 AM, Ben Pfaff wrote:
> On Tue, Jul 21, 2015 at 11:32:21AM -0700, Ethan Jackson wrote:
>> Thanks for the review, I've addressed the comments and will send out
>> another
g_create() when called from a secondary process. It also introduces
>> two
>> functions: netdev_dpdk_ring_rxq_recv() and netdev_dpdk_ring_send__() to
>> handle
>> tx/rx on dpdk rings in secondary processes.
>>
>> Signed-off-by: Melvin Walls
>> Signed-o
Sure, Melvin could you add that to the next version you send out
(after this version is reviewed).
Ethan
On Wed, Jul 22, 2015 at 4:31 PM, Ben Pfaff wrote:
> I understand now. I recommend adding a Co-authored-by, then, to make
> that clear.
>
> On Wed, Jul 22, 2015 at 04:29:55PM
, Jul 22, 2015 at 04:34:39PM -0700, Ethan Jackson wrote:
>> Sure, Melvin could you add that to the next version you send out
>> (after this version is reviewed).
>>
>> Ethan
>>
>> On Wed, Jul 22, 2015 at 4:31 PM, Ben Pfaff wrote:
>> > I understand now. I
Ben, Justin, should this be backported? I'm not up on the policy at the moment.
Etha
On Thu, Jul 23, 2015 at 3:42 AM, Traynor, Kevin wrote:
>
>> -Original Message-
>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di
>> Proietto
>> Sent: Thursday, July 16, 2015 7:48
think that the cause is this patch.
> When we tried branch-2.4, we ran into no errors.
> Today I am working on confirming that this is the patch.
>
> Luis E. P.
>
> Sent from my iPhone
>
>> On Jul 23, 2015, at 15:38, Ethan Jackson wrote:
>>
>> Ben, Justin,
Ah my bad sorry.
Acked-by: Ethan Jackson
On Tue, Jul 28, 2015 at 6:53 PM, Ben Pfaff wrote:
> Fixes the following error:
>
> The following files are in git but not the distribution:
> ovn/OVN-GW-HA.md
>
> CC: Ethan Jackson
> Signed-off-by: Ben Pfaff
> --
g
around vhost-user. My inclination would be to drop the patch instead
of implementing this logic unless there's consensus that IVSHMEM is
going to be an relevant IO mechanism in the future.
> However, maybe for your use-case this isn’t an issue?
Yeah for our (Melvin + Me) specific us
Sorry for taking so long to get to this. The one question I have is:
Is OVS the right layer to be fixing this? Isn't this really an issue
of DPDK reporting a number of available queues that for practical
purposes is wrong? I.E. Shouldn't this be fixed by the DPDK driver of
this system? This pat
thout something like the patch below.
> Not until we have more explicit configuration details available for the HW
> device from DPDK.
>
> Thanks
> Ian
>
>> -Original Message-
>> From: Ethan Jackson [mailto:et...@nicira.com]
>> Sent: Wednesday, J
Hey,
Really sorry this has taken so long to merge. I dropped the ball. It's in now.
Ethan
On Tue, Aug 4, 2015 at 5:26 AM, Ilya Maximets wrote:
> Will anyone plan to apply this patch?
>
> Best regards, Ilya Maximets.
>
> On 28.07.2015 23:48, Flavio Leitner wrote:
>> On Tue, Jul 28, 2015 at 09:
te up a note for theINSTALL.DPDK.md
> explaining the issue.
>
> Thanks
> Ian
>
>> -Original Message-
>> From: Ben Pfaff [mailto:b...@nicira.com]
>> Sent: Monday, August 03, 2015 6:11 PM
>> To: Ethan Jackson
>> Cc: Stokes, Ian; Justin Pettit; Pravin
t with
the MODIFY flag set.
Signed-off-by: Ethan Jackson
---
ofproto/ofproto-dpif-upcall.c | 118 +++---
1 file changed, 87 insertions(+), 31 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 59010c2..7abc97d 10
Great, I'll merge both today.
Thanks a lot,
Ethan
On Thu, Aug 6, 2015 at 9:00 AM, Stokes, Ian wrote:
> No problem,
>
> I've sent a patch to the mailing list with these changes for INSTALL.DPDK.md.
>
> Thanks
> Ian
>
>> -Original Message-
>> F
Merged and backported to 2.4. Thanks
Ethan
On Thu, Aug 6, 2015 at 11:23 AM, Ethan Jackson wrote:
> Great, I'll merge both today.
>
> Thanks a lot,
> Ethan
>
> On Thu, Aug 6, 2015 at 9:00 AM, Stokes, Ian wrote:
>> No problem,
>>
>> I've sent a pa
the bond rebalance test cases to prove that modify is used in
>> the case we currently know suffers from the lack of modify support?
>>
>> Also, there are some minor comments below you could address at the same time.
>>
>> Jarno
>>
>>> On Aug 5, 2015, at
>>> On Aug 10, 2015, at 6:46 PM, Ethan J. Jackson wrote:
>>>
>>> From: Ethan Jackson
>>>
>>> Since revalidator_sweep() doesn't hold the ukey mutex for each full
>>> loop iteration, it's theoretically possible that two threads ma
; revalidator/purge case, which could hit something like this, but I
> don't think there is a case where we use this functionality while OVS
> is under load, so the likelyhood of problems is low.
>
> On 11 August 2015 at 12:18, Ethan Jackson wrote:
>> Yeah sorry this patch
agreed, sent a new version.
Ethan
On Wed, Aug 12, 2015 at 5:01 PM, Jarno Rajahalme wrote:
>
>> On Aug 12, 2015, at 4:13 PM, Ethan J. Jackson wrote:
>>
>> From: Ethan Jackson
>>
>> Future patches will need to modify ukey actions in some instances.
>
2015, at 10:38 AM, Ethan J. Jackson wrote:
> >
> > From: Ethan Jackson
> >
> > Signed-off-by: Ethan J. Jackson
> > ---
> > AUTHORS | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/AUTHORS b/AUTHORS
> > index 4
Yeah, there's another Ethan Jackson at MSR unfortunately.
On Tue, Sep 1, 2015 at 10:42 AM, Ben Pfaff wrote:
> On Tue, Sep 01, 2015 at 10:38:26AM -0700, Ethan J. Jackson wrote:
>> From: Ethan Jackson
>>
>> Signed-off-by: Ethan J. Jackson
>> ---
>> AUTHORS
Acked-by: Ethan Jackson
On Thu, Apr 17, 2014 at 5:41 PM, Alex Wang wrote:
> Signed-off-by: Alex Wang
>
> ---
> V8 -> v9:
> - move the fat-lock acquisition inside the if-statement in dpif_linux_run().
> - fix an error: dpif_linux_recv_purge() should acquire wrlock.
>
Acked-by: Ethan Jackson
On Thu, Apr 17, 2014 at 5:41 PM, Alex Wang wrote:
> This commit reformats the dpif-linux module so that all internal
> static functions take 'struct dpif_linux *' as input argument.
> This will allow the adding of thread-safety annotations.
>
>
Acked-by: Ethan Jackson
On Thu, Apr 17, 2014 at 5:41 PM, Alex Wang wrote:
> Signed-off-by: Alex Wang
>
> ---
> V9:
> - add thread-safety annotations.
> ---
> lib/dpif-linux.c | 60
> +-
> 1 file changed, 46
used. */
> struct flow_miss *flow_miss;/* This upcall's flow_miss. */
Looks like is_valid isn't necessary anymore.
I was going to suggest some other major changes, but I think it
probably makes more sense to do it as a separate patch. Go ahead and
merge.
Acked-by: Ethan Jackson
Acked-by: Ethan Jackson
On Mon, Apr 21, 2014 at 6:13 PM, Alex Wang wrote:
> On current master, caller of udpif_set_threads() can pass 0 value
> on n_handlers and n_revalidators to delete all handler and revalidator
> threads.
>
> After commit 9a159f748866 (ofproto-dpif-upc
Acked-by: Ethan Jackson
On Mon, Apr 21, 2014 at 8:25 PM, Alex Wang wrote:
> Commit 1f8675481e (ofproto-dpif-upcall: Fix ovs-vswitchd crash.)
> directly copied the udpif_set_threads() logic to udpif_stop_threads()
> and udpif_start_threads(). In fact, this was erroneous and caused
&
est failures.
>
> This commit fixes the above issue by correcting the checks in
> udpif_stop_threads() and udpif_start_threads(), and adding necessary
> checks in udpif_set_threads().
>
> Acked-by: Ethan Jackson
> Signed-off-by: Alex Wang
>
> ---
> PATCH -> V2:
> -
> I don't understand why, if you need to know when vswitchd restarts (why do
> you?), it should be tied to netdevs.
So if you're monitoring a netdev for packet stats, you need to know if
vswitchd restarted so that you can take account of the fact the stats
were reset. I suppose it needs to be tie
Though we've discussed it, at this point there is no public bug
tracker for the OVS project. We just moved to github, so there may be
one there, but to date we haven't used it.
If you want to see what went into the 1.10 release, you may consider
checking the 1.10 git log for the 1.10 branch. We
The problem has actually gotten worse since we've gotten rid of the
dispatcher thread. Now each thread has it's own channel per port.
I wonder if the right approach is to simply ditch the per-port
fairness in the case where mmap netlink is enabled. I.E. we simply
have one channel per thread and
Acked-by: Ethan Jackson
On Wed, Apr 23, 2014 at 1:26 PM, Andy Zhou wrote:
> This patch improves the code readability and comments on the
> recirculation related changes to rule_dpif_lookup() base on off-line
> discussions with Jarno. There is no behavior changes.
>
> Signed-of
Acked-by: Ethan Jackson
On Wed, Apr 23, 2014 at 4:20 PM, Jarno Rajahalme wrote:
> This allows rules to be used without taking references while RCU
> protected.
>
> The last step of destroying an ofproto also needs to be postponed, as
> the rule destruction requires the class
Traditionally we've put function comments just in the .c file, not in
the .c and .h file as you've done for rule_dpif_lookup(). That said,
I don't know where that convention came from and don't care that much
. . .
Acked-by: Ethan Jackson
On Wed, Apr 23, 2014 at 4:20
LGTM
Acked-by: Ethan Jackson
On Wed, Apr 23, 2014 at 4:20 PM, Jarno Rajahalme wrote:
> Only take reference to a looked up rule when needed.
>
> This reduces the total CPU utilization of rule_ref/unref calls by 80%,
> from 5% of total server CPU capacity to 1% in a netperf TC
Thanks for taking care of this.
Acked-by: Ethan Jackson
On Wed, Apr 23, 2014 at 5:06 PM, Ben Pfaff wrote:
> Before commit 2a73b1d73d4bdb (bridge: Reconfigure in single pass.), if a
> port disappeared, for one reason or another, from a datapath, the next
> bridge reconfiguration p
M +1200, Joe Stringer wrote:
>> > From: Ethan Jackson
>> >
>> > Previously, we had a separate flow_dumper thread that fetched flows from
>> > the datapath to distribute to revalidator threads. This patch takes the
>> > logic for dumping and pushes it i
_add(hash, daddr[0]);
> +hash = mhash_add(hash, daddr[1]);
> +hash = mhash_add(hash, daddr[2]);
> +hash = mhash_add(hash, daddr[3]);
Maybe use a loop?
Acked-by: Ethan Jackson
___
dev mailing list
dev@openvswitch.org
htt
The first line in the commit message needs a period.
As Yamamoto asked, please verify that this actually helps. If it
doesn't I'd prefer not to merge it.
I think we need to get rid of the hindex anyways, so I'd prefer we
don't inline it now.
Assuming the above is address
Acked-by: Ethan Jackson
On Fri, Apr 18, 2014 at 12:41 PM, Jarno Rajahalme wrote:
> This was the only place in OVS code that accessed classifier internal
> data structures directly. Use the classifier cursor API instead, so
> that following patches can hide classifier internal data s
So it seems to me that the only data needed in struct classifier by
callers is the fat_rwlock. What if we add a classifier_rdlock() and a
classifier_wrlock() function. That done, we could entirely hide
struct classifier, and would need to make the distinction between it
and the cls_classifier. T
Acked-by: Ethan Jackson
On Fri, Apr 18, 2014 at 12:41 PM, Jarno Rajahalme wrote:
> We only need to iterate over the bits masked by the 'b' in
> minimask_has_extra(), since for zeroes in 'b' there can be no 'extra'
> wildcards in 'a&
I think there should be a comment explaining when to use prefetch vs
prefetch_write. I certainly don't know off the top of my head.
Acked-by: Ethan Jackson
On Fri, Apr 18, 2014 at 12:41 PM, Jarno Rajahalme wrote:
> Define OVS_PREFETCH() and OVS_PREFETCH_WRITE() using builtin prefet
In the cache_push_back function, you might consider using the
x2nrealloc() function.
Does this actually help? I've found we spend most of our time getting
the memory for the rule, not the subtable itself.
Ethan
On Fri, Apr 18, 2014 at 12:41 PM, Jarno Rajahalme wrote:
> Using a linear array all
I haven't read this patch yet, but a high level question. Why not
just hide all of struct cls_rule and make callers embed a pointer to
it? We'd replace the cls_rule_init() function with a
cls_rule_create() function which mallocs the rule and returns it (for
example).
Ethan
On Fri, Apr 18, 2014
Acked-by: Ethan Jackson
I don't really like the API we've ended up with here. But I think
it's fine for now. As discussed offline, I intend to change it later.
Ethan
On Thu, Apr 24, 2014 at 6:25 PM, Ethan Jackson wrote:
> I haven't read this patch yet, but a high l
Acked-by: Ethan Jackson
I'm not sure I totally follow Kmindg's comment, perhaps he could
explain further? At any rate, once addressed I"m happy with this.
Ethan
On Sat, Apr 19, 2014 at 10:09 PM, Kmindg G wrote:
> On Sat, Apr 19, 2014 at 3:42 AM, Jarno Rajahalme
> wrot
The comment of miniflow_and_mask_matches_flow() need to be reworded.
Do we really need that function? Is it really that much faster?
Having two functions which do the exact same thing is a bit confusing.
So I suppose the only reason for a non zero inline_values array is for
those callers who can
Fine with me go ahead and push it.
Ethan
On Tue, Apr 29, 2014 at 12:40 PM, Jarno Rajahalme wrote:
>
> On Apr 24, 2014, at 5:40 PM, Ethan Jackson wrote:
>
>> So it seems to me that the only data needed in struct classifier by
>> callers is the fat_rwlock. What if we ad
> The common use case is a flow expiring during the lifetime of a TCP
> connection. It will result in multiple data packets being sent upwards.
> It's much less likely in the megaflows era though.
I think this is pretty unlikely except in some extreme circumstances.
For a flow to be removed from t
Sounds good.
Ethan
On Tue, Apr 29, 2014 at 12:58 PM, Jarno Rajahalme wrote:
>
> On Apr 24, 2014, at 6:15 PM, Ethan Jackson wrote:
>
>> In the cache_push_back function, you might consider using the
>> x2nrealloc() function.
>>
>
> I did not know this existed
> The almost equivalent function miniflow_equal_flow_in_minimask() does not
> require the miniflow and minimask to have the same map, and it is slower for
> it. However, it is only used by the test-classifier.c, so one option to
> reduce the confusion would be to move it there. What do you think
Meh I changed my mind again. Just merge the series instead of holding
back, I'll make my tweaks on top of master.
Ethan
On Tue, Apr 29, 2014 at 2:34 PM, Ethan Jackson wrote:
>> The almost equivalent function miniflow_equal_flow_in_minimask() does not
>> require the miniflo
1 - 100 of 3563 matches
Mail list logo
