[ovs-dev] [PATCH] dpif-netdev: Free packets on TUNNEL_PUSH if may_steal.

[Ilya Maximets]

On 15.05.2018 20:18, Ben Pfaff wrote:
> On Tue, May 15, 2018 at 02:23:38PM +0300, Ilya Maximets wrote:
>> Unconditional return may cause packet leak in case of
>> 'may_steal == true'.
>>
>> Additionally, removed redundant checking for depth level and
>> clarified ignoring of the 'false' value of 'may_steal'.
>>
>> CC: Sugesh Chandran <sugesh.chandran at intel.com>
>> Fixes: 7c12dfc527a5 ("tunneling: Avoid datapath-recirc by
>>                       combining recirc actions at xlate.")
>> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> 
> Thanks.  This seems reasonable to me.
> 
> Did you take a look at the other cases in the function to see whether
> they have the same problem?

I reviewed other cases inside 'dp_execute_cb()' and they seems correct,
at least in terms of freeing the packets.

---
It's not related to current patch, but one thing that I found is that
OVS_ACTION_ATTR_CT unconditionally modifies the packets regardless of
'may_steal' value. For example it could update ip addresses for NAT
purposes inside 'conntrack_execute()'.
Is it acceptable? Are we always in kind of cloned state while excuting CT?
I think this should be fixed or clarified in code by comments, because
this breaks the API of 'dp_execute_action()'.

Best regards, Ilya Maximets.