On Tue, Mar 03, 2015 at 11:22:43AM -0800, Ben Pfaff wrote:
> I need to continue reading this patch starting from
> ofproto-dpif-upcall.c.
Continuing:
ofproto-dpif-upcall.c
---------------------
s/deleate/delete/:
+ * relies on OVS userspace internal state, we need to deleate all old
ofproto-dpif-upcall.h
---------------------
It might be nice to add a helper to report whether the recircs are
inline, since all the existing helpers check that.
Any particular reason the new recirc_id_node parameter to
xlate_actions() isn't part of xlate_in?
ofproto-dpif-xlate.c
--------------------
This commit introduces much more extensive use of the 'exit' member of
xlate_ctx. Until now, I think the usage was simple: if the 'exit'
action was encountered, or some action had something like a "fatal
error", we set 'exit', which stopped translating actions. Now there's
something more subtle going on that I don't understand yet. Can you
tell me a little more about it?
There's what looks like a really complicated dance going on with
recirc_action_offset and adding actions to the action set. I'm having
trouble figuring out what's going on. So far, it appears to me that
it's basically unrolling the OpenFlow invocation call stack, but
that's not really clear to me. Can you explain a little bit?
In struct xlate_ctx, the passive voice here forces the reader to guess
what code must do the associating. I think it probably means that
xlate_actions()'s caller must do it, but it would be nice to say that:
+ /* These are used for non-bond recirculation. The recirculation IDs are
+ * stored in xout and must be associated with a datapath flow (ukey),
+ * otherwise they will be freed when the xout is uninitialized. */
+ int recirc_action_offset; /* Offset in 'action_set' to actions to be
+ * executed after recirculation, or -1. */
+ int last_unroll_offset; /* Offset in 'action_set' to the latest unroll
+ * action, or -1. */
This comment in compose_output_action__() jumped out at me. Why do
you think that the action set should be executed before outputting to
a patch port?
+ /* XXX: Action set should be executed (xlate_action_set()) before
+ * output to a patch port? */
In xlate_actions(), the use of - here instead of != or > looks odd to me:
+ /* Restore recirculation actions. These include unroll actions to
+ * restore the pipeline metadata. */
+ if (recirc->ofpacts_len - recirc->action_set_Len) {
I need to read more but I think I need some more explanations first.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
