> -----Original Message----- > From: Legacy, Allain [mailto:allain.leg...@windriver.com] > Sent: Wednesday, April 11, 2018 12:28 PM > To: Ananyev, Konstantin <konstantin.anan...@intel.com> > Cc: dev@dpdk.org; Peters, Matt (Wind River) <matt.pet...@windriver.com>; > sta...@dpdk.org > Subject: RE: [PATCH v2] ip_frag: fix double free of chained mbufs > > > -----Original Message----- > > From: Ananyev, Konstantin [mailto:konstantin.anan...@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;
Ah ok, makes sense. > > > > 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. If we already doing that as you pointed above, then probably no need for new solution. Konstantin
