ofproto-dpif: Fix tag caching for learned flows.
authorBen Pfaff <blp@nicira.com>
Tue, 20 Mar 2012 22:26:57 +0000 (15:26 -0700)
committerBen Pfaff <blp@nicira.com>
Wed, 21 Mar 2012 16:03:51 +0000 (09:03 -0700)
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@nicira.com>
ofproto/ofproto-dpif.c

index 2afd94a9cdc69cca85c9c66d6be149b8537c1090..86b56567cfb972cdaa40397f0f7178d96c8359e7 100644 (file)
@@ -4414,7 +4414,7 @@ xlate_table_action(struct action_xlate_ctx *ctx,
         if (table_id > 0 && table_id < N_TABLES) {
             struct table_dpif *table = &ofproto->tables[table_id];
             if (table->other_table) {
         if (table_id > 0 && table_id < N_TABLES) {
             struct table_dpif *table = &ofproto->tables[table_id];
             if (table->other_table) {
-                ctx->tags |= (rule
+                ctx->tags |= (rule && rule->tag
                               ? rule->tag
                               : rule_calculate_tag(&ctx->flow,
                                                    &table->other_table->wc,
                               ? rule->tag
                               : rule_calculate_tag(&ctx->flow,
                                                    &table->other_table->wc,