[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