odp-util: Do not set port mask of non-IP packets
authorSimon Horman <horms@verge.net.au>
Wed, 4 Jun 2014 16:56:09 +0000 (16:56 +0000)
committerJarno Rajahalme <jrajahalme@nicira.com>
Wed, 4 Jun 2014 16:16:43 +0000 (09:16 -0700)
commit791a09be15d3cd36f6a59086794a6ef2b4a76669
treed291535610e4b131811299f1f361bda704745b80
parent4377ac06063c91b7df5550a9b35217e98bf1f56d
odp-util: Do not set port mask of non-IP packets

In the case that an flow for an IP packet has an mpls_push action applied
the L3 and L4 portions of the flow will be cleared in flow_push_mpls().

Without this change commit_set_port_action() will set the tp_src and tp_dst
mask for the flow to all-ones because the base and flow port values no
longer match. Even though there will be no corresponding set action for the
ports; because the flow is no longer IP.

In this case where nw_proto is not part of the match this manifests
in a problem because the kernel datapath rejects flows whose masks
have non-zero values for tp_src or dp_dst if the nw_proto mask is
not all-ones.

This patch resolves this problem by having commit_set_port_action() return
without doing anything if flow->nw_proto is zero. The same logic is present
in commit_set_nw_action().

Also enhance one of the MPLS tests to exercise this logic.  The enhanced
tests inputs a UDP packet with non-zero ports rather than an IP packet with
zeroed ports: zeroed ports cause commit_set_port_action() always return
without doing anything..

Commit 691d39b ("upcall: Remove redundant xlate_actions_for_side_effects().")
causes xlate_in_init() to be called for every packet that has an upcall.
This has the effect of indirectly calling commit_set_port_action() when
translating a controller action which may not have previously been the case
depending on the flow.

The result is that the behaviour described in the changelog above can be
exercised via a minor enhancement to one of the existing MPLS tests. This
illustrates that the problem exists for the user-space datapath whereas I
had previously incorrectly assumed it only manifested when using the kernel
datapath because I had only observed it there.

Signed-off-by: Simon Horman <horms@verge.net.au>
Acked-by: Jarno Rajahalme <jrajahame@nicira.com>
lib/odp-util.c
tests/ofproto-dpif.at