On Wed, Mar 20, 2013 at 6:18 AM, Simon Horman <ho...@verge.net.au> wrote: > * Update the order in which actions are committed and thus > appear in the datapath such that MPLS actions appear after > l3 and l4 (nw and port) actions. > > In the case where an mpls_push action is present it should ensure > that l3 and l4 actions occur first, which seems logical as > once a mpls_push has occur the frame will be MPLS rather > than IPv4 or IPv6. > > In the case where there is an mpls_pop action is present this should > not make any difference as the frame will have been MPLS to start with > and thus not satisfy the pre-requisites for l3 or l4 actions. > > * Update commit_set_nw_action() to use the base ethertype when considering > eligibility to commit l3 (nw) actions. This allows l3 actions to be > applied so long as the frame was originally IPv4 or IPv6, even if > an mpls_push action will be applied and thus flow indicates the > frame will be MPLS. > > * Make actions that may modify port or network information conditional on > the flow's ethernet type being an IP ethernet type. This is to ensure > that actions that modify network and port information do not occur > on non IP packets, for example if an mpls_push action has changed a > packet from IP to MPLS. > > Note that modification of network data is already prevented by > virtue of commit_set_nw_action() only having cases for when the > ethernet type of the flow is IPV4 or IPV6. The conditionality > of network actions on the ethernet type has been added to > do_xlate_actions() to make it explicit. > > Signed-off-by: Simon Horman <ho...@verge.net.au>
Applied, thanks. I realized that there is one more possible case that could cause problems here: if something like reg_move is used to load a value into the transport port field without using one of the field specific actions. Completely fixing this seemed like more trouble than it is worth for the time being, so I essentially combined the two strategies - the one you have here to give nice behavior in the common case and blocking it at the commit_set_port_action() level to provide final protection. The idea that you had earlier of just checking that the base flow is IP seemed cleanest, so I just did that. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev