[ovs-git] Open vSwitch: ofproto-dpif: Fix tag caching for learned flows. (branch-1.4)
dev at openvswitch.org
dev at openvswitch.org
Wed Mar 21 16:25:30 UTC 2012
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Open vSwitch".
The branch, branch-1.4 has been updated
via 69ccab125a23c9d1d1ee523cedf7df9dca1dd700 (commit)
via 19fc14ad08b4bc79515a52cc1a89b6f6e92225a3 (commit)
via f4f9f0868c9150c7b032884f212db1e3bcb6b1fa (commit)
via eef4d7399be65ab0a8d75b3eb448c4c91eb651a9 (commit)
from 9f9120615d9dc3bf030d965e21852a59167e0441 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 69ccab125a23c9d1d1ee523cedf7df9dca1dd700
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=69ccab125a23c9d1d1ee523cedf7df9dca1dd700
Author: Ben Pfaff <blp at nicira.com>
ofproto-dpif: Fix tag caching for learned flows.
This code in xlate_table_action() is supposed to tag flows in tables that
have special forms so that changes do not require revalidating every flow.
When rule->tag is nonzero, its value can be used, because we know in this
case that rule->cr.wc is the same as table->other_table->wc and that thus
rule->tag caches the return value of the rule_calculate_tag() expression.
When rule->tag is zero (a "catchall" rule) we need to calculate the tag
manually because we have no way to cache it in that case.
I discovered this bug by running an "hping3" between a couple of VMs plus
the following commands on OVS in the middle:
ovs-ofctl del-flows br0
ovs-ofctl add-flow br0 "table=0 actions=learn(table=1, \
idle_timeout=600, NXM_OF_VLAN_TCI[0..11], \
NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[], \
output:NXM_OF_IN_PORT[], fin_idle_timeout=10), resubmit(,1)"
ovs-ofctl add-flow br0 "table=1 priority=0 actions=flood"
Without this patch, flows don't get properly invalidated upon initial MAC
learning, so one sees warnings like the following:
in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),
eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,
ttl=64,frag=no),tcp(src=13966,dst=0): inconsistency in subfacet
(actions were: 3,0,1) (correct actions: 1)
This patch fixes the problem and thus avoids these warnings.
Signed-off-by: Ben Pfaff <blp at nicira.com>
commit 19fc14ad08b4bc79515a52cc1a89b6f6e92225a3
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=19fc14ad08b4bc79515a52cc1a89b6f6e92225a3
Author: Ben Pfaff <blp at nicira.com>
ofproto-dpif: Avoid segfault deleting facets that execute LEARN actions.
"ovs-ofctl del-flows <bridge>" can result in the following call path:
delete_flows_loose() in ofproto.c
-> collect_rules_loose() -- uses 'ofproto_node' inside 'struct rule'
-> rule_destruct() in ofproto-dpif.c
-> facet_revalidate()
-> facet_remove()
-> facet_flush_stats()
-> facet_account()
-> xlate_actions()
-> xlate_learn_action()
-> ofproto_flow_mod() back in ofproto.c
-> modify_flow_strict()
-> collect_rules_strict() -- also uses 'ofproto_node'
which goes "boom" when we fall back up the call chain because the nested
use of ofproto_node steps on the outer use of ofproto_node.
This commit fixes the problem by refusing to translate "learn" actions
within facet_flush_stats(), breaking the doubled use.
Another possible approach would be to switch to another way to keep track
of rules in the flow_mod implementations, so that there'd be no fighting
over 'ofproto_node'. But then "ovs-ofctl del-flows" might still leave some
flows around (ones created by "learn" actions as flows are accounted as
facets get deleted), which would be surprising behavior. And it seems in
general a bad idea to allow recursive flow_mods; the consequences have not
been carefully thought through.
Before this commit, one can reproduce the problem by running an "hping3"
between a couple of VMs plus the following commands on OVS in the middle.
Sometimes you have to run them a few times:
ovs-ofctl del-flows br0
ovs-ofctl add-flow br0 "table=0 actions=learn(table=1, \
idle_timeout=600, NXM_OF_VLAN_TCI[0..11], \
NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[], \
output:NXM_OF_IN_PORT[], fin_idle_timeout=10), resubmit(,1)"
ovs-ofctl add-flow br0 "table=1 priority=0 actions=flood"
This commit has a side effect that leftover unaccounted packets no longer
update the timeouts in MAC learning actions in some cases, when the facets
that cause updates are deleted. At most one second of updates should be
lost.
Bug #10184.
Reported-by: Michael Mao <mmao at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
commit f4f9f0868c9150c7b032884f212db1e3bcb6b1fa
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=f4f9f0868c9150c7b032884f212db1e3bcb6b1fa
Author: Ben Pfaff <blp at nicira.com>
hmap: New function hmap_contains().
This is useful in a situation where one knows that an hmap_node is in some
hmap, but it's not certain which one, and one needs to know whether it is
in a particular one. This is not a very common case; I don't see any
potential users in the current tree, although an upcoming commit will add
one.
Signed-off-by: Ben Pfaff <blp at nicira.com>
commit eef4d7399be65ab0a8d75b3eb448c4c91eb651a9
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=eef4d7399be65ab0a8d75b3eb448c4c91eb651a9
Author: Ben Pfaff <blp at nicira.com>
ofproto-dpif: Fix return type of rule_calculate_tag().
tag_type is currently uint32_t but using uint32_t directly is conceptually
wrong.
Signed-off-by: Ben Pfaff <blp at nicira.com>
-----------------------------------------------------------------------
Summary of changes:
lib/hmap.c | 17 ++++++++++++++++-
lib/hmap.h | 4 +++-
ofproto/ofproto-dpif.c | 44 +++++++++++++++++++++++++++-----------------
3 files changed, 46 insertions(+), 19 deletions(-)
hooks/post-receive
--
Open vSwitch
More information about the git
mailing list