> From: Xin Long [mailto:lucien....@gmail.com] > Sent: Wednesday, May 3, 2017 1:38 PM > On Wed, May 3, 2017 at 10:07 AM, Gao Feng <gfree.w...@foxmail.com> > wrote: > >> From: netdev-ow...@vger.kernel.org > >> [mailto:netdev-ow...@vger.kernel.org] > >> On Behalf Of Xin Long > >> Sent: Wednesday, May 3, 2017 12:59 AM On Tue, May 2, 2017 at 7:03 PM, > >> Gao Feng <gfree.w...@vip.163.com> wrote: > >> >> From: Xin Long [mailto:lucien....@gmail.com] > >> >> Sent: Tuesday, May 2, 2017 3:56 PM On Sat, Apr 29, 2017 at 11:51 > >> >> AM, <gfree.w...@foxmail.com> wrote: > >> >> > From: Gao Feng <gfree.w...@foxmail.com> [...] > > The fix you mentioned change the original logic. > > The dev->vstats is freed in advance in the ndo_uninit, not destructor. > > It may break the backward. > Sorry, I didn't get your "backward" > I can't see there will be any problem caused by it. > can you say this patch also break the 'backward' ? > https://patchwork.ozlabs.org/patch/748964/ > > It's really weird to do dev->reg_state check in ndo_unint ndo_unint is > supposed > to free the memory alloced in ndo_init. >
I am not sure if it would break the backward, so I said it MAY break. I assumed there may be someone would access the dev->vstats after ndo_uninit, because current veth driver free the mem in the destructor. I selected this approach because I don't want to bring new bugs during fix bug. If you're sure it is safe to free dev->vstats in ndo_uninit, I would like to update it. BTW there are too many drivers which have possible memleak. You could find the list by https://www.mail-archive.com/netdev@vger.kernel.org/msg166629.html. Some drivers allocate the resources in ndo_init, free some in ndo_uninit and free left in destructor. I think there are some reasons. We could not move all free in the ndo_uninit from destructor. What's your opinion? Best Regards Feng
