On Tue, May 08, 2018 at 12:34:54AM +0530, Anju Thomas wrote:
> 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.tho...@ericsson.com>

Thanks for the patch.  It applies OK and all the tests pass.

It looks like your commit message describes at least two other bugs in
OVS, though.  First, if OVS crashes when it pushes tunnel headers, even
if there's not enough headroom, that's really bad.  At worst, it should
drop the packet.  Do you know where the crash occurs?  We should fix the
problem, since it might recur in some other context.

Second, it's a little shocking to hear that an encapsulation action
without a following output action causes a memory leak.  We also need to
fix that.  Do you have any more details?

Thanks,

Ben.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to