Skip to content

Commit

Permalink
Fix crash due to multiple tnl push action
Browse files Browse the repository at this point in the history
During slow path packet processing, if the action is to output to a
tunnel port, the slow path processing of the encapsulated packet
continues on the underlay bridge and additional actions (e.g. optional
VLAN encapsulation, bond link selection and finally output to port) are
collected there.

To prepare for a continuation of the processing of the original packet
(e.g. output to other tunnel ports in a flooding scenario), the
“tunnel_push” action and the actions of the underlay bridge are
encapsulated in a clone() action to preserve the original packet.

If the underlay bridge decides to drop the tunnel packet (for example if
both bonded ports are down simultaneously), the clone(tunnel_push))
actions previously generated as part of translation of the output to
tunnel port are discarded and a stand-alone tunnel_push action is added
instead. Thus the tunnel header is pushed on to the original packet.
This is the bug.

Consequences: If packet processing continues with sending to further
tunnel ports, multiple tunnel header pushes will happen on the original
packet as typically the tunnels all traverse the same underlay bond
which is down. The packet may not have enough headroom to accommodate
all the tunnel headers. OVS crashes if it runs out of space while trying
to push the tunnel headers.

Even in case there is enough headroom, the packet will not be freed
since the accumulated action list contains only the tunnel header push
action without any output port action. Thus, we either have a crash or a
packet buffer leak.

Signed-off-by: Anju Thomas <anju.thomas@ericsson.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
Anju Thomas authored and blp committed Jan 22, 2019
1 parent dffacff commit 4b51f86
Showing 1 changed file with 0 additions and 7 deletions.
7 changes: 0 additions & 7 deletions ofproto/ofproto-dpif-xlate.c
Expand Up @@ -3510,13 +3510,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport,
nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
} else {
nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
/* XXX : There is no real use-case for a tunnel push without
* any post actions. However keeping it now
* as is to make the 'make check' happy. Should remove when all the
* make check tunnel test case does something meaningful on a
* tunnel encap packets.
*/
odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
}

/* Restore context status. */
Expand Down

0 comments on commit 4b51f86

Please sign in to comment.