On Tue, 25 Jun 2019 at 13:12, Roopa Prabhu <ro...@cumulusnetworks.com> wrote: >
Hi Roopa, Thank you for the review! > On Sun, Jun 23, 2019 at 7:18 PM Taehee Yoo <ap420...@gmail.com> wrote: > > > > On Mon, 24 Jun 2019 at 03:07, David Miller <da...@davemloft.net> wrote: > > > > > > > Hi David, > > > > Thank you for the review! > > > > > From: Taehee Yoo <ap420...@gmail.com> > > > Date: Thu, 20 Jun 2019 20:51:08 +0900 > > > > > > > __vxlan_dev_create() destroys FDB using specific pointer which indicates > > > > a fdb when error occurs. > > > > But that pointer should not be used when register_netdevice() fails > > > > because > > > > register_netdevice() internally destroys fdb when error occurs. > > > > > > > > In order to avoid un-registered dev's notification, fdb destroying > > > > routine > > > > checks dev's register status before notification. > > > > > > Simply pass do_notify as false in this failure code path of > > > __vxlan_dev_create(), > > > thank you. > > > > Failure path of __vxlan_dev_create() can't handle do_notify in that case > > because if register_netdevice() fails it internally calls > > ->ndo_uninit() which is > > vxlan_uninit(). > > vxlan_uninit() internally calls vxlan_fdb_delete_default() and it callls > > vxlan_fdb_destroy(). > > do_notify of vxlan_fdb_destroy() in vxlan_fdb_delete_default() is always > > true. > > So, failure path of __vxlan_dev_create() doesn't have any opportunity to > > handle do_notify. > > > I don't see register_netdevice calling ndo_uninit in case of all > errors. In the case where it does not, > does your patch leak the fdb entry ?. > > Wondering if we should just use vxlan_fdb_delete_default with a notify > flag to delete the entry if exists. > Will that help ? > > There is another commit that touched this code path: > commit 6db9246871394b3a136cd52001a0763676563840 > > Author: Petr Machata <pe...@mellanox.com> > Date: Tue Dec 18 13:16:00 2018 +0000 > vxlan: Fix error path in __vxlan_dev_create() I have checked up failure path of register_netdevice(). Yes, this patch leaks fdb entry. There are 3 failure cases in the register_netdevice(). A. error occurs before calling ->ndo_init(). it doesn't call ->ndo_uninit(). B. error occurs after calling ->ndo_init(). it calls ->ndo_uninit() and dev->reg_state is NETREG_UNINITIALIZED. C. error occurs after registering netdev. it calls rollback_registered(). rollback_registered() internally calls ->ndo_uninit() and dev->reg_state is NETREG_UNREGISTERING. A panic due to these problem could be fixed by using vxlan_fdb_delete_default() with notify flag. But notification problem could not be fixed clearly because of the case C. I don't have clear solution for the case C. Please let me know, if you have any good idea for fixing the case C. Thank you!
