Hi,
I've tested with your fix (865f22b3b3cb9), and it solves the problem
indeed. My approach to fix the problem is based on the comment of the
tag member:
/* A hash bucket for mapping a flow to a slave.
* "struct bond" has an array of (BOND_MASK + 1) of these. */
struct bond_entry {
struct bond_slave *slave; /* Assigned slave, NULL if unassigned. */
...
tag_type tag; /* Tag for entry<->slave association. */
My understanding was that the hash is associated to the slave by this
field, however it doesn't make too much sense as 'slave' already do
this. Your commit message on 865f22b3b3cb9 cleared the picture, I
suggest to clarify this comment for bond_entry:tag.
My solution also works but it forces ofproto to revalidate every other
hash on the same slave, which is not necessary as their slave did not
change. You can drop it.
Regards,
Zoli
On 31/12/12 17:14, Ben Pfaff wrote:
Oh, also you said you tested it against OVS 1.4.2, but not against
1.4.3, which has an important relevant fix as commit 865f22b3b3cb9
(bond: Tag flows according to their hash bucket, not just their slave.).
It's always best to test against the latest version in whatever branch
you're using.
On Mon, Dec 31, 2012 at 09:12:11AM -0800, Ben Pfaff wrote:
The patch doesn't entirely add up. I've been intending to test the
behavior myself, but with the holidays and other stuff I have to do, I
haven't managed it yet.
On Mon, Dec 31, 2012 at 03:48:12PM +0000, Zoltan Kiss wrote:
Hi,
Did you guys had any chance to check this patch? (and the one
related in the next mail)
Btw. the numbering in the subject is wrong, it should be [PATCH 1/2].
Zoli
On 20/12/12 21:39, Zoltan Kiss wrote:
As the description of this field states, it should link the hash entry
to the slave. With random tags, the flow revalidation never finds the
corresponding facet after bond rebalancing, therefore the flow
entries never get updated.
I've tested it on Xenserver 6.1 (OVS 1.4.2), two NIC with LACP bonding
and 4 VMs running iperf client against iperf servers running on two
separate hosts on the same Ethernet switch (1 Gbps connection each).
Running a similar command like the following during test
running helps you to see whether the flow entries followed the
rebalancing or not:
ovs-dpctl dump-flows xapi1|grep dst=5001; ovs-appctl bond/show bond0
Signed-off-by: Zoltan Kiss <zoltan.k...@citrix.com>
---
lib/bond.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/bond.c b/lib/bond.c
index 2c59f9d..ba98fff 100644
--- a/lib/bond.c
+++ b/lib/bond.c
@@ -739,7 +739,7 @@ bond_shift_load(struct bond_entry *hash, struct bond_slave
*to,
/* Arrange for flows to be revalidated. */
tag_set_add(set, hash->tag);
hash->slave = to;
- hash->tag = tag_create_random();
+ hash->tag = to->tag;
}
/* Pick and returns a bond_entry to migrate to 'to' (the least-loaded slave),
@@ -1446,7 +1446,7 @@ choose_output_slave(const struct bond *bond, const struct
flow *flow,
if (!e->slave->enabled) {
e->slave = bond->active_slave;
}
- e->tag = tag_create_random();
+ e->tag = e->slave->tag;
}
*tags |= e->tag;
return e->slave;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
