On Wed, May 3, 2017 at 2:37 PM, Gao Feng [off-list ref] wrote:

quoted

From: Xin Long [mailto:lucien.xin@gmail.com]
Sent: Wednesday, May 3, 2017 1:38 PM
On Wed, May 3, 2017 at 10:07 AM, Gao Feng [off-list ref]
wrote:

quoted

quoted

From: netdev-owner@vger.kernel.org
[mailto:netdev-owner@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 [off-list ref] wrote:

quoted

quoted

From: Xin Long [mailto:lucien.xin@gmail.com]
Sent: Tuesday, May 2, 2017 3:56 PM On Sat, Apr 29, 2017 at 11:51
AM,  [off-list ref] wrote:

quoted

From: Gao Feng <redacted>

[...]

quoted

quoted

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.

yes, stats are accessed in .ndo_start_xmit waited by synchronize_net() and
.ndo_get_stats64 protected by rtnl_lock().

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.

ah, cool.
I'm not sure about other dev's stuff, have to check them for sure later.

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