Re: [PATCH net v2] net: bareudp: add missing error handling for bareudp_link_config()

On Tue, 5 Jan 2021 12:38:54 -0800 Cong Wang wrote:

On Tue, Jan 5, 2021 at 11:07 AM Jakub Kicinski [off-list ref] wrote:

quoted

+static void bareudp_dellink(struct net_device *dev, struct list_head *head)
+{
+       struct bareudp_dev *bareudp = netdev_priv(dev);
+
+       list_del(&bareudp->next);
+       unregister_netdevice_queue(dev, head);
+}
+
 static int bareudp_newlink(struct net *net, struct net_device *dev,
                           struct nlattr *tb[], struct nlattr *data[],
                           struct netlink_ext_ack *extack)
 {
        struct bareudp_conf conf;
+       LIST_HEAD(list_kill);
        int err;

        err = bareudp2info(data, &conf, extack);

@@ -662,17 +671,14 @@ static int bareudp_newlink(struct net *net, struct net_device *dev,

        err = bareudp_link_config(dev, tb);
        if (err)
-               return err;
+               goto err_unconfig;

        return 0;
-}
-
-static void bareudp_dellink(struct net_device *dev, struct list_head *head)
-{
-       struct bareudp_dev *bareudp = netdev_priv(dev);

-       list_del(&bareudp->next);
-       unregister_netdevice_queue(dev, head);
+err_unconfig:
+       bareudp_dellink(dev, &list_kill);
+       unregister_netdevice_many(&list_kill);  

Why do we need unregister_netdevice_many() here? I think
bareudp_dellink(dev, NULL) is sufficient as we always have
one instance to unregister?

(For the same reason, bareudp_dev_create() does not need it
either.)

Ack, I'm following how bareudp_dev_create() is written. 

I can follow up in net-next and change both, sounds good?