On Jun 5, 2013, at 10:43 PM, Ben Pfaff <b...@nicira.com> wrote:
> Justin, I know that you're eager for a full review of this patch, but
> it's just not going to happen tonight. I have some initial feedback
> here, and I'll finish it up tomorrow morning.
No problem. I appreciate you continuing to look at it so late tonight.
I agree with your comments and your interpretation of the structures and their
behavior. Sorry the comments were not as thorough as I'd have liked, but they
got dropped in the rush to get the code out (with the plan to add them in the
next revision). I'll address that in v2.
> Have you tried doing "make check-valgrind" on at least some tests to
> check for memory leaks (and bugs)? It's a little time consuming
> (takes several minutes, maybe half an hour? with TESTSUITEFLAGS=-j8)
> but it can be useful especially for large changes.
Yes, I had been running it on the new xout tests. I expanded it to all the
ofproto-dpif tests, and it found a problem with the trace command. I've
appended a patch that fixes the issue. (And it was even a problem I
specifically pointed out in a new comment!)
Let me know how the rest of the patch goes, and I'll turn around a new version
quickly.
Thanks again!
--Justin
-=-=-=-=-=-=-=-=-=-=-=-=-
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index d126ea0..b367c9b 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5055,6 +5055,8 @@ facet_check_consistency(struct facet *facet)
/* Check the datapath actions for consistency. */
xlate_in_init(&xin, ofproto, &facet->flow, &facet->initial_vals, rule,
0, NULL);
+
+ flow_wildcards_init_catchall(&xout.wc);
xlate_actions(&xin, &xout);
ok = ofpbuf_equal(&facet->xc->xout.odp_actions, &xout.odp_actions)
@@ -5753,6 +5755,7 @@ rule_dpif_execute(struct rule_dpif *rule, const struct flo
xlate_in_init(&xin, ofproto, flow, &initial_vals, rule, stats.tcp_flags,
packet);
xin.resubmit_stats = &stats;
+ flow_wildcards_init_catchall(&xout.wc);
xlate_actions(&xin, &xout);
execute_odp_actions(ofproto, flow, xout.odp_actions.data,
@@ -5812,6 +5815,8 @@ send_packet(const struct ofport_dpif *ofport, struct ofpbu
xin.ofpacts_len = sizeof output;
xin.ofpacts = &output.ofpact;
xin.resubmit_stats = &stats;
+
+ flow_wildcards_init_catchall(&xout.wc);
xlate_actions(&xin, &xout);
error = dpif_execute(ofproto->backer->dpif,
@@ -7262,6 +7267,7 @@ xlate_actions_for_side_effects(struct xlate_in *xin)
{
struct xlate_out xout;
+ flow_wildcards_init_catchall(&xout.wc);
xlate_actions(xin, &xout);
xlate_out_uninit(&xout);
}
@@ -8162,6 +8168,7 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet
xin.ofpacts_len = ofpacts_len;
xin.ofpacts = ofpacts;
+ flow_wildcards_init_catchall(&xout.wc);
xlate_actions(&xin, &xout);
dpif_execute(ofproto->backer->dpif, key.data, key.size,
xout.odp_actions.data, xout.odp_actions.size, packet);
@@ -8549,6 +8556,8 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct f
packet);
trace.xin.resubmit_hook = trace_resubmit;
trace.xin.report_hook = trace_report;
+
+ flow_wildcards_init_catchall(&trace.xout.wc);
xlate_actions(&trace.xin, &trace.xout);
ds_put_char(ds, '\n');
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
