Skip to content

Commit

Permalink
ofproto-dpif: Make ofproto/trace a bit more like real packet translat…
Browse files Browse the repository at this point in the history 
…ion.

Until now, ofproto/trace has looked up the flow itself.  xlate_actions()
can do the flow lookup internally and, since that is what happens when a
packet arrives, having it do its own packet lookup makes a lot of sense.

I noticed this in connection with the actset_output field, which
xlate_actions() should set to OFPP_UNSET at the beginning of translation
before looking up the flow.  ofproto/trace didn't do that, so it looked
up a rule with actset_output=0 instead.  By having xlate_actions() do the
lookup, the behavior can be consistent and correct.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
  • Loading branch information
@blp
blp committed Nov 4, 2014
1 parent 5859b0a commit a8c3134
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 56 deletions.
20 changes: 18 additions & 2 deletions ofproto/ofproto-dpif-xlate.c
View file
Expand Up @@ -2746,8 +2746,8 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
ctx->xin->resubmit_stats);
ctx->xin->flow.in_port.ofp_port = old_in_port;

if (ctx->xin->resubmit_hook) {
ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse);
if (OVS_UNLIKELY(ctx->xin->resubmit_hook)) {
ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse + 1);
}

switch (verdict) {
Expand Down Expand Up @@ -4256,6 +4256,18 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
!xin->skip_wildcards ? wc : NULL,
&rule, ctx.xin->xcache != NULL,
ctx.xin->resubmit_stats);
if (OVS_UNLIKELY(ctx.xin->report_hook)) {
if (rule == ctx.xbridge->miss_rule) {
xlate_report(&ctx, "No match, flow generates \"packet in\"s.");
} else if (rule == ctx.xbridge->no_packet_in_rule) {
xlate_report(&ctx, "No match, packets dropped because "
"OFPPC_NO_PACKET_IN is set on in_port.");
} else if (rule == ctx.xbridge->drop_frags_rule) {
xlate_report(&ctx, "Packets dropped because they are IP "
"fragments and the fragment handling mode is "
"\"drop\".");
}
}
if (ctx.xin->resubmit_stats) {
rule_dpif_credit_stats(rule, ctx.xin->resubmit_stats);
}
Expand All @@ -4266,6 +4278,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
entry->u.rule = rule;
}
ctx.rule = rule;

if (OVS_UNLIKELY(ctx.xin->resubmit_hook)) {
ctx.xin->resubmit_hook(ctx.xin, rule, 0);
}
}
xout->fail_open = ctx.rule && rule_dpif_is_fail_open(ctx.rule);

Expand Down
89 changes: 35 additions & 54 deletions ofproto/ofproto-dpif.c
View file
Expand Up @@ -4410,11 +4410,13 @@ trace_resubmit(struct xlate_in *xin, struct rule_dpif *rule, int recurse)
struct ds *result = trace->result;

ds_put_char(result, '\n');
trace_format_flow(result, recurse + 1, "Resubmitted flow", trace);
trace_format_regs(result, recurse + 1, "Resubmitted regs", trace);
trace_format_odp(result, recurse + 1, "Resubmitted odp", trace);
trace_format_megaflow(result, recurse + 1, "Resubmitted megaflow", trace);
trace_format_rule(result, recurse + 1, rule);
if (recurse) {
trace_format_flow(result, recurse, "Resubmitted flow", trace);
trace_format_regs(result, recurse, "Resubmitted regs", trace);
trace_format_odp(result, recurse, "Resubmitted odp", trace);
trace_format_megaflow(result, recurse, "Resubmitted megaflow", trace);
}
trace_format_rule(result, recurse, rule);
}

static void
Expand Down Expand Up @@ -4701,7 +4703,6 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
const struct ofpact ofpacts[], size_t ofpacts_len,
struct ds *ds)
{
struct rule_dpif *rule;
struct trace_ctx trace;

ds_put_format(ds, "Bridge: %s\n", ofproto->up.name);
Expand All @@ -4710,65 +4711,45 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
ds_put_char(ds, '\n');

flow_wildcards_init_catchall(&trace.wc);
if (ofpacts) {
rule = NULL;
} else {
rule_dpif_lookup(ofproto, flow, &trace.wc, &rule, false, NULL);

trace_format_rule(ds, 0, rule);
if (rule == ofproto->miss_rule) {
ds_put_cstr(ds, "\nNo match, flow generates \"packet in\"s.\n");
} else if (rule == ofproto->no_packet_in_rule) {
ds_put_cstr(ds, "\nNo match, packets dropped because "
"OFPPC_NO_PACKET_IN is set on in_port.\n");
} else if (rule == ofproto->drop_frags_rule) {
ds_put_cstr(ds, "\nPackets dropped because they are IP fragments "
"and the fragment handling mode is \"drop\".\n");
}
}

if (rule || ofpacts) {
trace.result = ds;
trace.key = flow; /* Original flow key, used for megaflow. */
trace.flow = *flow; /* May be modified by actions. */
xlate_in_init(&trace.xin, ofproto, flow, flow->in_port.ofp_port, rule,
ntohs(flow->tcp_flags), packet);
if (ofpacts) {
trace.xin.ofpacts = ofpacts;
trace.xin.ofpacts_len = ofpacts_len;
}
trace.xin.resubmit_hook = trace_resubmit;
trace.xin.report_hook = trace_report;
trace.result = ds;
trace.key = flow; /* Original flow key, used for megaflow. */
trace.flow = *flow; /* May be modified by actions. */
xlate_in_init(&trace.xin, ofproto, flow, flow->in_port.ofp_port, NULL,
ntohs(flow->tcp_flags), packet);
trace.xin.ofpacts = ofpacts;
trace.xin.ofpacts_len = ofpacts_len;
trace.xin.resubmit_hook = trace_resubmit;
trace.xin.report_hook = trace_report;

xlate_actions(&trace.xin, &trace.xout);
xlate_actions(&trace.xin, &trace.xout);

ds_put_char(ds, '\n');
trace_format_flow(ds, 0, "Final flow", &trace);
trace_format_megaflow(ds, 0, "Megaflow", &trace);
ds_put_char(ds, '\n');
trace_format_flow(ds, 0, "Final flow", &trace);
trace_format_megaflow(ds, 0, "Megaflow", &trace);

ds_put_cstr(ds, "Datapath actions: ");
format_odp_actions(ds, ofpbuf_data(trace.xout.odp_actions),
ofpbuf_size(trace.xout.odp_actions));
ds_put_cstr(ds, "Datapath actions: ");
format_odp_actions(ds, ofpbuf_data(trace.xout.odp_actions),
ofpbuf_size(trace.xout.odp_actions));

if (trace.xout.slow) {
enum slow_path_reason slow;
if (trace.xout.slow) {
enum slow_path_reason slow;

ds_put_cstr(ds, "\nThis flow is handled by the userspace "
"slow path because it:");
ds_put_cstr(ds, "\nThis flow is handled by the userspace "
"slow path because it:");

slow = trace.xout.slow;
while (slow) {
enum slow_path_reason bit = rightmost_1bit(slow);
slow = trace.xout.slow;
while (slow) {
enum slow_path_reason bit = rightmost_1bit(slow);

ds_put_format(ds, "\n\t- %s.",
slow_path_reason_to_explanation(bit));
ds_put_format(ds, "\n\t- %s.",
slow_path_reason_to_explanation(bit));

slow &= ~bit;
}
slow &= ~bit;
}

xlate_out_uninit(&trace.xout);
}

xlate_out_uninit(&trace.xout);
}

/* Store the current ofprotos in 'ofproto_shash'. Returns a sorted list
Expand Down

0 comments on commit a8c3134

Please sign in to comment.