Francesco Ruggeri <frugg...@arista.com> writes: > If macvlan_common_newlink fails in register_netdevice after macvlan_init > then it decrements port->count twice, first in macvlan_uninit (from > register_netdevice or rollback_registered) and then again in > macvlan_common_newlink. > A similar problem may exist in the ipvlan driver. > This patch consolidates modifications to port->count into macvlan_init > and macvlan_uninit (thanks to Eric Biederman for suggesting this approach). > In macvtap_device_event it also avoids cleaning up in NETDEV_UNREGISTER > if NETDEV_REGISTER had previously failed. > > Signed-off-by: Francesco Ruggeri <frugg...@arista.com>
The macvlan_init bits obviously corect. The macvtap bits I don't really understand what is going on. > diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c > index 95394ed..e770221 100644 > --- a/drivers/net/macvtap.c > +++ b/drivers/net/macvtap.c > @@ -1303,6 +1303,8 @@ static int macvtap_device_event(struct notifier_block > *unused, > } > break; > case NETDEV_UNREGISTER: > + if (vlan->minor == 0) > + break; I don't understand this bit. A minor of 0 is never assigned. That is clear from the code. On what code path can you get here without assigning a minor? My gut says this either needs a big fat comment explaining what is going on, or a slight refactoring of the code (like moving port count increments into macvlan_init) that makes it so this code path can't happen. > devt = MKDEV(MAJOR(macvtap_major), vlan->minor); > device_destroy(macvtap_class, devt); > macvtap_free_minor(vlan); Eric
