On Sun, Dec 29, 2013 at 2:50 AM, Ben Pfaff <b...@nicira.com> wrote: > I've been a little frustrated with the current approach to MPLS, because it > seems quite difficult to understand. One particularly difficult bit for > me is the variables used during translation, e.g. mpls_depth_delta and > pre_push_mpls_lse. And what we end up with is support for a single MPLS > label, which I don't think is going to make any real-world users happy. > > This commit attempts to implement something easier to understand and more > powerful by just keeping track of all the labels in struct flow. > > Signed-off-by: Ben Pfaff <b...@nicira.com> > Co-authored-by: Simon Horman <ho...@verge.net.au> > Signed-off-by: Simon Horman <ho...@verge.net.au>
I have some general comments: * I'm not sure that flow_count_mpls_labels() is megaflow-safe since it reads fields without updating the wildcards. I think the only real way to handle this is with automatic tracking. * It seems like we might want a harder failure mechanism for cases where we exceed the number of labels that we are able to handle. I don't think that this is the same as trying to change L4 ports when you don't have an L4 header because an MPLS packet could still be interpreted, just differently. Other things, assuming that the goal is to eventually hook this up to the kernel patches: * How do we handle cases where userspace supports parsing more labels than the kernel (which is actually true at the moment)? I think that it would just fail at flow installation time right now. * Somewhat related to the above, the kernel's action validator doesn't trust userspace to provide a valid EtherType for pop actions, so it doesn't allow multiple consecutive pops in cases where there are more tags than it knows about (which is 1). _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
