On Fri, May 16, 2014 at 11:30:23AM +0900, Simon Horman wrote: > In some cases an pop MPLS action changes a packet to be a non-mpls packet. > In this case subsequent any L3+ actions require access to portions > of the packet which were not decoded as they were opaque when the > packet was MPLS. Allow such actions to be translated by > first recirculating the packet. > > Signed-off-by: Simon Horman <ho...@verge.net.au>
The first three patches look fine to me. compose_recirculate_action() in this one makes me worry a bit about a corner case. If there is no rule, then compose_recirculate_action() allocates one. I think that in the common case, there is a rule. But a "packet_out" message does translate actions without a rule. I think that means that each "packet_out" from a controller can create unnecessarily create a recirculation id and corresponding rule that will take a long time to expire and never get reused. If I'm right about that, then one solution would be to do recirculation in userspace in the case where we have a packet and no rule. That's a pretty nasty corner case, though (do we ever want to do that in other circumstances?) and so for now I'd be happy with a comment in the ofproto_dpif_alloc_recirc_id() case in compose_recirculate_action() explaining what's going on. I think that, for now, recirculation will lose metadata fields like registers. That's not a new problem, and I believe that Andy Zhou (or maybe Justin Pettit?) is working on a general solution, so I'm not asking you to fix it, but I think that it also bears mention in a comment. I don't understand may_xlate_l3_actions. It seems to cover a lot more than L3, e.g. VLANs and Goto-Table. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
