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 <[email protected]> >> Fixes: 7c12dfc527a5 ("tunneling: Avoid datapath-recirc by >> combining recirc actions at xlate.") >> Signed-off-by: Ilya Maximets <[email protected]> > > 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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
