Re: [dpdk-dev] [PATCH v2] ip_frag: fix double free of chained mbufs

["Legacy, Allain" <Allain.Legacy@windriver.com>]

> -----Original Message-----
> From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com]
> Sent: Wednesday, April 11, 2018 7:02 AM
<..>
> 
> 
> I wonder why we have to NULL only first and cur entry?
> We probably have to NULL each one in that case, right?

We have to do first and current entries at those locations because 
the code does not clear them properly.  All other entries are cleared by 
the following piece of code but it does not handle the two cases that I am 
addressing with my change.

	/* this mbuf should not be accessed directly */
	fp->frags[curr_idx].mb = NULL;
	curr_idx = i;


> If so, then it probably better to do in the same place we do
> ip_frag_key_invalidate().

I don't feel that ip_frag_key_invalidate is the appropriate place to take care of this.  In the interest of code readability and maintainability it should stick to what its name implies and only invalidate the key and nothing else.    Since the ipv*_frag_reassemble() functions were already setup to set the pointers to NULL it should continue to be done there, but of course since it is does not handle all cases correctly it should be fixed.


> As alternative, and probably better approach - can we modify
> rte_ip_frag_table_destroy(), so it will free mbufs only for entires with valid
> keys?

If you prefer this approach I can start over. 

Allain