diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index a426fcfeb..8723cb4e8 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5974,14 +5974,36 @@ freeze_unroll_actions(const struct ofpact *a, const struct ofpact *end, } break; + /* From an OpenFlow point of view, goto_table and write_metadata are + * instructions, not actions. This means that to use them, we'd have + * to reformulate the actions as instructions, which is possible, and + * we'd have slot them into the frozen actions in a specific order, + * which doesn't seem practical. Instead, we translate these + * instructions into equivalent actions. */ + case OFPACT_GOTO_TABLE: { + struct ofpact_resubmit *resubmit + = ofpact_put_RESUBMIT(&ctx->frozen_actions); + resubmit->in_port = OFPP_IN_PORT; + resubmit->table_id = ofpact_get_GOTO_TABLE(a)->table_id; + resubmit->with_ct_orig = false; + } + continue; + case OFPACT_WRITE_METADATA: { + const struct ofpact_metadata *md = ofpact_get_WRITE_METADATA(a); + const struct mf_field *mf = mf_from_id(MFF_METADATA); + ovs_assert(mf->n_bytes == sizeof md->metadata); + ovs_assert(mf->n_bytes == sizeof md->mask); + ofpact_put_set_field(&ctx->frozen_actions, mf, + &md->metadata, &md->mask); + } + continue; + case OFPACT_SET_TUNNEL: case OFPACT_REG_MOVE: case OFPACT_SET_FIELD: case OFPACT_STACK_PUSH: case OFPACT_STACK_POP: case OFPACT_LEARN: - case OFPACT_WRITE_METADATA: - case OFPACT_GOTO_TABLE: case OFPACT_ENQUEUE: case OFPACT_SET_VLAN_VID: case OFPACT_SET_VLAN_PCP: @@ -6911,16 +6933,21 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, } break; + /* Freezing complicates resubmit and goto_table. Some action in the + * flow entry found by resubmit might trigger freezing. If that + * happens, then we do not want to execute the resubmit or goto_table + * again after during thawing, so we want to skip back to the head of + * the loop to avoid that, only adding any actions that follow the + * resubmit to the frozen actions. + */ case OFPACT_RESUBMIT: - /* Freezing complicates resubmit. Some action in the flow - * entry found by resubmit might trigger freezing. If that - * happens, then we do not want to execute the resubmit again after - * during thawing, so we want to skip back to the head of the loop - * to avoid that, only adding any actions that follow the resubmit - * to the frozen actions. - */ xlate_ofpact_resubmit(ctx, ofpact_get_RESUBMIT(a), last); continue; + case OFPACT_GOTO_TABLE: + xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port, + ofpact_get_GOTO_TABLE(a)->table_id, + true, true, false, last, do_xlate_actions); + continue; case OFPACT_SET_TUNNEL: flow->tunnel.tun_id = htonll(ofpact_get_SET_TUNNEL(a)->tun_id); @@ -7089,17 +7116,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, xlate_meter_action(ctx, ofpact_get_METER(a)); break; - case OFPACT_GOTO_TABLE: { - struct ofpact_goto_table *ogt = ofpact_get_GOTO_TABLE(a); - - ovs_assert(ctx->table_id < ogt->table_id); - - xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port, - ogt->table_id, true, true, false, last, - do_xlate_actions); - break; - } - case OFPACT_SAMPLE: xlate_sample_action(ctx, ofpact_get_SAMPLE(a)); break; diff --git a/tests/nsh.at b/tests/nsh.at index e84134e42..b958be253 100644 --- a/tests/nsh.at +++ b/tests/nsh.at @@ -368,8 +368,8 @@ bridge("br0") ------------- thaw Resuming from table 0 - Restoring actions: goto_table:2 - goto_table:2 + Restoring actions: resubmit(,2) + resubmit(,2) 2. packet_type=(1,0x894f),nsh_mdtype=1,nsh_spi=0x1234,nsh_c1=0x11223344, priority 32768 decap() diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 956a69e1f..ed3b141b3 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -5568,7 +5568,8 @@ dnl dnl EXTRA_SETUP is an optional list of extra commands to run after setting up dnl both bridges, e.g. to configure mirrors or patch ports. m4_define([CHECK_CONTINUATION], [dnl - AT_SETUP([ofproto-dpif - continuation - $1]) + m4_foreach([monitor_version], [[OpenFlow10], [OpenFlow13]], [dnl + AT_SETUP([ofproto-dpif - continuation - $1 - monitor_version]) AT_KEYWORDS([continuations pause resume]) OVS_VSWITCHD_START @@ -5587,10 +5588,10 @@ m4_define([CHECK_CONTINUATION], [dnl add_of_ports --pcap br1 `seq m4_eval([$2 + 1]) m4_eval([$2 + $3])`]) AT_CAPTURE_FILE([ofctl_monitor0.log]) - AT_CHECK([ovs-ofctl monitor br0 resume --detach --no-chdir --pidfile=ovs-ofctl0.pid 2> ofctl_monitor0.log]) + AT_CHECK([ovs-ofctl -O monitor_version monitor br0 resume --detach --no-chdir --pidfile=ovs-ofctl0.pid 2> ofctl_monitor0.log]) m4_if([$3], [0], [], [AT_CAPTURE_FILE([ofctl_monitor1.log]) - AT_CHECK([ovs-ofctl monitor br1 resume --detach --no-chdir --pidfile=ovs-ofctl1.pid 2> ofctl_monitor1.log])]) + AT_CHECK([ovs-ofctl -O monitor_version monitor br1 resume --detach --no-chdir --pidfile=ovs-ofctl1.pid 2> ofctl_monitor1.log])]) actions0='$4' actions1='$5' @@ -5614,9 +5615,19 @@ m4_define([CHECK_CONTINUATION], [dnl AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$flow"], [0], [stdout]) # Wait for the expected number of packets to show up. - n_packets=`expr $n_packets + $2 - 1 + $3` - echo "waiting for $n_packets packets..." - OVS_WAIT_UNTIL([test $n_packets = `ovs-ofctl parse-pcap p*-tx.pcap | wc -l`]) + n_packets=$(expr $n_packets + 1) + nports=m4_eval([$2 + $3]) + echo "waiting for $n_packets packets on p2 through p$nports..." + OVS_WAIT_UNTIL([ + ok=true + for i in $(seq 2 $nports); do + n=$(ovs-ofctl parse-pcap p$i-tx.pcap | wc -l) + printf "%d " $n + if test $n_packets != $n; then ok=false; fi + done + echo + $ok + ]) # Wait for the expected number of NXT_RESUMEs to be logged. n_resumes=$(expr $n_resumes + $(echo "$actions0 $actions1" | count_matches pause) ) @@ -5651,6 +5662,7 @@ m4_define([CHECK_CONTINUATION], [dnl done OVS_VSWITCHD_STOP AT_CLEANUP + ]) ]) # Check that pause at the end of the pipeline works OK. @@ -5680,6 +5692,22 @@ CHECK_CONTINUATION([action set], [3], [0], [in_port=1 actions=1 pause resubmit(,1) pause 2 table=1 actions=write_actions(3)]) +# Check that goto_table instructions following pause work as expected. +CHECK_CONTINUATION([goto_table], [6], [0], + [table=0 in_port=1 actions=pause 2 pause goto_table:1 + table=1 in_port=1 actions=pause 3 pause goto_table:2 + table=2 in_port=1 actions=pause 4 pause goto_table:3 + table=3 in_port=1 actions=pause 5 pause goto_table:4 + table=4 in_port=1 actions=pause 6 pause]) + +# Check that write_metadata instructions following pause work as expected. +CHECK_CONTINUATION([write_metadata], [6], [0], + [table=0 in_port=1 actions=pause 2 pause write_metadata:1/1 goto_table:1 + table=1 in_port=1 metadata=1 actions=pause 3 pause write_metadata:2/2 goto_table:2 + table=2 in_port=1 metadata=3 actions=pause 4 pause write_metadata:4/4 goto_table:3 + table=3 in_port=1 metadata=7 actions=pause 5 pause write_metadata:8/8 goto_table:4 + table=4 in_port=1 metadata=15 actions=pause 6 pause]) + # Check that metadata and the stack used by push and pop is preserved # across pause/resume. CHECK_CONTINUATION([data stack], [3], [0],