[ovs-dev] [PATCH 2/3] ofproto-dpif: Make ofproto/trace a bit more like real packet translation.
Ben Pfaff
blp at nicira.com
Tue Nov 4 01:14:04 UTC 2014
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 at nicira.com>
---
ofproto/ofproto-dpif-xlate.c | 18 +++++++++-
ofproto/ofproto-dpif.c | 77 +++++++++++++++---------------------------
2 files changed, 45 insertions(+), 50 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8a8eb92..a6bde1d 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2747,7 +2747,7 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
ctx->xin->flow.in_port.ofp_port = old_in_port;
if (ctx->xin->resubmit_hook) {
- ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse);
+ ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse + 1);
}
switch (verdict) {
@@ -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);
}
@@ -4266,6 +4278,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
entry->u.rule = rule;
}
ctx.rule = rule;
+
+ if (ctx.xin->resubmit_hook) {
+ ctx.xin->resubmit_hook(ctx.xin, rule, 0);
+ }
}
xout->fail_open = ctx.rule && rule_dpif_is_fail_open(ctx.rule);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 744850c..a4f10e4 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4703,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);
@@ -4712,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
--
1.7.10.4
More information about the dev
mailing list