[ovs-dev] [PATCH 1/2] mpls: Allow l3 and l4 actions to prior to a push_mpls action

Jesse Gross jesse at nicira.com
Wed Mar 20 22:31:51 UTC 2013


On Wed, Mar 20, 2013 at 6:18 AM, Simon Horman <horms at 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 <horms at 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.



More information about the dev mailing list