On Wed, May 16, 2018 at 6:01 AM, Ilya Maximets <[email protected]> wrote:
> 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()'. > Thanks for pointing that out Ilya. I'll propose something to address it. Darrell > > Best regards, Ilya Maximets. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
