On Mon, Dec 16, 2013 at 09:01:37AM -0800, Ben Pfaff wrote: > On Mon, Dec 16, 2013 at 06:42:23PM +0900, Simon Horman wrote: > > On Mon, Dec 16, 2013 at 02:55:39PM +0900, Simon Horman wrote: > > > Hi Ben, > > > > > > On Fri, Dec 13, 2013 at 12:28:21AM -0800, Ben Pfaff 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. > > > > > > > > I think that we can do something easier to understand and more powerful > > > > by just keeping track of all the labels in struct flow. This patch is a > > > > first stab at that. It's incomplete--in particular I have not > > > > implemented > > > > commit_mpls_action() because it seems slightly tricky and it's very late > > > > at night here--and obviously untested, but I'd appreciate feedback on > > > > the > > > > approach. > > > > > > Its quite a bit of code to digest but in general my feeling > > > at this point is that this approach could be made to work. > > > > > > There are a number of corner cases that my patchset (the previous > > > approach) > > > tries to address. And I believe they contributed to most of the more > > > difficult to understand code. > > > > > > I have tried mentally running through a number of them to see how they > > > might work out with this new approach. One that seems slightly tricky > > > is the following in the case that MPLS LSEs are being pushed before VLAN > > > tags. > > > > > > Assuming no VLAN is present in the original packet: > > > > > > push_vlan,push_mpls,push_vlan > > > > > > Or, alternatively, if a VLAN is present in the original packet: > > > > > > set_vlan_vid,push_mpls,push_vlan > > > > > > I believe that with this new approach the first VLAN action would be > > > silently dropped. I believe that this case could be handled using > > > recirculation and in the mean time detected and rejected. But I wanted to > > > bring it to your attention to see how you feel about it. > > > > > > > I think that if this works out then it would obviate the need for > > > > patches > > > > "odp: Allow VLAN actions after MPLS actions" and "lib: Support pushing > > > > of > > > > MPLS LSE before or after VLAN tag". > > > > > > Yes, that seems likely. > > > > One thing I forgot in my previous email, would you like me to > > work on polishing up this patch? > > I'd like to complete my work, and then it's fair game for anyone to > shoot holes in it or improve it.
That sounds reasonable. I hope we can focus on the improve rather than the holes :) And if you do need any help please let me know. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
