notifiers: Create and destroy nln_notifiers.

This patch changes the interface of netlink-notifier and
rtnetlink-link.  Now nln_notifiers are allocated and destroyed by
the module instead of passed in by callers.  This allows the
definition of nln_notifier to be hidden, and generally cleans up
the code.

lib/dpif-linux.c

@@ -137,7 +137,7 @@ struct dpif_linux {
 
     /* Change notification. */
     struct sset changed_ports;  /* Ports that have changed. */
-    struct nln_notifier port_notifier;
+    struct nln_notifier *port_notifier;
     bool change_error;
 
     /* Queue of unused ports. */
@@ -253,13 +253,12 @@ static int
 open_dpif(const struct dpif_linux_dp *dp, struct dpif **dpifp)
 {
     struct dpif_linux *dpif;
-    int error;
     int i;
 
     dpif = xmalloc(sizeof *dpif);
-    error = nln_notifier_register(nln, &dpif->port_notifier,
+    dpif->port_notifier = nln_notifier_create(nln, dpif_linux_port_changed,
-                                  dpif_linux_port_changed, dpif);
+                                              dpif);
-    if (error) {
+    if (!dpif->port_notifier) {
         goto error_free;
     }
 
@@ -286,7 +285,7 @@ open_dpif(const struct dpif_linux_dp *dp, struct dpif **dpifp)
 
 error_free:
     free(dpif);
-    return error;
+    return EINVAL;
 }
 
 static void
@@ -294,10 +293,7 @@ dpif_linux_close(struct dpif *dpif_)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
 
-    if (nln) {
+    nln_notifier_destroy(dpif->port_notifier);
-        nln_notifier_unregister(nln, &dpif->port_notifier);
-    }
-
     nl_sock_destroy(dpif->mc_sock);
     sset_destroy(&dpif->changed_ports);
     free(dpif->lru_bitmap);

lib/netdev-linux.c

@@ -105,7 +105,7 @@ COVERAGE_DEFINE(netdev_ethtool);
 #define TC_RTAB_SIZE 1024
 #endif
 
-static struct nln_notifier netdev_linux_cache_notifier;
+static struct nln_notifier *netdev_linux_cache_notifier = NULL;
 static int cache_notifier_refcount;
 
 enum {
@@ -526,13 +526,15 @@ netdev_linux_create(const struct netdev_class *class, const char *name,
                     struct netdev_dev **netdev_devp)
 {
     struct netdev_dev_linux *netdev_dev;
-    int error;
 
     if (!cache_notifier_refcount) {
-        error = rtnetlink_link_notifier_register(&netdev_linux_cache_notifier,
+        assert(!netdev_linux_cache_notifier);
-                                                 netdev_linux_cache_cb, NULL);
+
-        if (error) {
+        netdev_linux_cache_notifier =
-            return error;
+            rtnetlink_link_notifier_create(netdev_linux_cache_cb, NULL);
+
+        if (!netdev_linux_cache_notifier) {
+            return EINVAL;
         }
     }
     cache_notifier_refcount++;
@@ -622,7 +624,9 @@ netdev_linux_destroy(struct netdev_dev *netdev_dev_)
         cache_notifier_refcount--;
 
         if (!cache_notifier_refcount) {
-            rtnetlink_link_notifier_unregister(&netdev_linux_cache_notifier);
+            assert(netdev_linux_cache_notifier);
+            rtnetlink_link_notifier_destroy(netdev_linux_cache_notifier);
+            netdev_linux_cache_notifier = NULL;
         }
     } else if (class == &netdev_tap_class) {
         destroy_tap(netdev_dev);

lib/netlink-notifier.c

@@ -18,6 +18,7 @@
 
 #include "netlink-notifier.h"
 
+#include <assert.h>
 #include <errno.h>
 #include <poll.h>
 #include <stdlib.h>
@@ -46,6 +47,14 @@ struct nln {
     void *change;                /* Change passed to parse. */
 };
 
+struct nln_notifier {
+    struct nln *nln;             /* Parent nln. */
+
+    struct list node;
+    nln_notify_func *cb;
+    void *aux;
+};
+
 /* Creates an nln handle which may be used to manage change notifications.  The
  * created handle will listen for netlink messages on 'multicast_group' using
  * netlink protocol 'protocol' (e.g. NETLINK_ROUTE, NETLINK_GENERIC, ...).
@@ -70,11 +79,15 @@ nln_create(int protocol, int multicast_group, nln_parse_func *parse,
 }
 
 /* Destroys 'nln' by freeing any memory it has reserved and closing any sockets
- * it has opened. */
+ * it has opened.
+ *
+ * The caller is responsible for destroying any notifiers created by this
+ * 'nln' before destroying 'nln'. */
 void
 nln_destroy(struct nln *nln)
 {
     if (nln) {
+        assert(list_is_empty(&nln->all_notifiers));
         nl_sock_destroy(nln->notify_sock);
         free(nln);
     }
@@ -87,11 +100,12 @@ nln_destroy(struct nln *nln)
  * This is probably not the function you want.  You should probably be using
  * message specific notifiers like rtnetlink_link_notifier_register().
  *
- * Returns 0 if successful, otherwise a positive errno value. */
+ * Returns an initialized nln_notifier if successful, otherwise NULL. */
-int
+struct nln_notifier *
-nln_notifier_register(struct nln *nln, struct nln_notifier *notifier,
+nln_notifier_create(struct nln *nln, nln_notify_func *cb, void *aux)
-                      nln_notify_func *cb, void *aux)
 {
+    struct nln_notifier *notifier;
+
     if (!nln->notify_sock) {
         struct nl_sock *sock;
         int error;
@@ -103,7 +117,7 @@ nln_notifier_register(struct nln *nln, struct nln_notifier *notifier,
         if (error) {
             nl_sock_destroy(sock);
             VLOG_WARN("could not create netlink socket: %s", strerror(error));
-            return error;
+            return NULL;
         }
         nln->notify_sock = sock;
     } else {
@@ -112,22 +126,29 @@ nln_notifier_register(struct nln *nln, struct nln_notifier *notifier,
         nln_run(nln);
     }
 
+    notifier = xmalloc(sizeof *notifier);
     list_push_back(&nln->all_notifiers, &notifier->node);
     notifier->cb = cb;
     notifier->aux = aux;
-    return 0;
+    notifier->nln = nln;
+    return notifier;
 }
 
-/* Cancels notification on 'notifier', which must have previously been
+/* Destroys 'notifier', which must have previously been created with
- * registered with nln_notifier_register(). */
+ * nln_notifier_register(). */
 void
-nln_notifier_unregister(struct nln *nln, struct nln_notifier *notifier)
+nln_notifier_destroy(struct nln_notifier *notifier)
 {
+    if (notifier) {
+        struct nln *nln = notifier->nln;
+
         list_remove(&notifier->node);
         if (list_is_empty(&nln->all_notifiers)) {
             nl_sock_destroy(nln->notify_sock);
             nln->notify_sock = NULL;
         }
+        free(notifier);
+    }
 }
 
 /* Calls all of the registered notifiers, passing along any as-yet-unreported

lib/netlink-notifier.h

@@ -23,6 +23,8 @@
 #include "list.h"
 
 struct nln;
+struct nln_notifier;
+
 struct nlattr;
 struct ofpbuf;
 
@@ -37,18 +39,12 @@ typedef void nln_notify_func(const void *change, void *aux);
  * should be parsed into 'change' as specified in nln_create(). */
 typedef bool nln_parse_func(struct ofpbuf *buf, void *change);
 
-struct nln_notifier {
-    struct list node;
-    nln_notify_func *cb;
-    void *aux;
-};
-
 struct nln *nln_create(int protocol, int multicast_group, nln_parse_func *,
                        void *change);
 void nln_destroy(struct nln *);
-int nln_notifier_register(struct nln *, struct nln_notifier *,
+struct nln_notifier *nln_notifier_create(struct nln *, nln_notify_func *,
-                          nln_notify_func *, void *aux);
+                                         void *aux);
-void nln_notifier_unregister(struct nln *, struct nln_notifier *);
+void nln_notifier_destroy(struct nln_notifier *);
 void nln_run(struct nln *);
 void nln_wait(struct nln *);
 #endif /* netlink-notifier.h */

lib/route-table.c

@@ -69,8 +69,8 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
 static unsigned int register_count = 0;
 static struct nln *nln = NULL;
 static struct route_table_msg rtmsg;
-static struct nln_notifier route_notifier;
+static struct nln_notifier *route_notifier = NULL;
-static struct nln_notifier name_notifier;
+static struct nln_notifier *name_notifier = NULL;
 
 static bool route_table_valid = false;
 static bool name_table_valid = false;
@@ -162,11 +162,14 @@ route_table_register(void)
 {
     if (!register_count) {
         assert(!nln);
+        assert(!route_notifier);
 
         nln = nln_create(NETLINK_ROUTE, RTNLGRP_IPV4_ROUTE,
                          (nln_parse_func *) route_table_parse, &rtmsg);
-        nln_notifier_register(nln, &route_notifier,
+
-                              (nln_notify_func *) route_table_change, NULL);
+        route_notifier =
+            nln_notifier_create(nln, (nln_notify_func *) route_table_change,
+                                NULL);
 
         hmap_init(&route_map);
         route_table_reset();
@@ -396,14 +399,15 @@ static void
 name_table_init(void)
 {
     hmap_init(&name_map);
-    rtnetlink_link_notifier_register(&name_notifier, name_table_change, NULL);
+    name_notifier = rtnetlink_link_notifier_create(name_table_change, NULL);
     name_table_valid = false;
 }
 
 static void
 name_table_uninit(void)
 {
-    rtnetlink_link_notifier_unregister(&name_notifier);
+    rtnetlink_link_notifier_destroy(name_notifier);
+    name_notifier = NULL;
     name_map_clear();
     hmap_destroy(&name_map);
 }

lib/rtnetlink-link.c

@@ -85,25 +85,24 @@ rtnetlink_link_parse_cb(struct ofpbuf *buf, void *change)
  * using dpif_port_poll() or netdev_change_seq(), which unlike this function
  * are not Linux-specific.
  *
- * Returns 0 if successful, otherwise a positive errno value. */
+ * Returns an initialized nln_notifier if successful, NULL otherwise. */
-int
+struct nln_notifier *
-rtnetlink_link_notifier_register(struct nln_notifier *notifier,
+rtnetlink_link_notifier_create(rtnetlink_link_notify_func *cb, void *aux)
-                                 rtnetlink_link_notify_func *cb, void *aux)
 {
     if (!nln) {
         nln = nln_create(NETLINK_ROUTE, RTNLGRP_LINK, rtnetlink_link_parse_cb,
                          &rtn_change);
     }
 
-    return nln_notifier_register(nln, notifier, (nln_notify_func *) cb, aux);
+    return nln_notifier_create(nln, (nln_notify_func *) cb, aux);
 }
 
-/* Cancels notification on 'notifier', which must have previously been
+/* Destroys 'notifier', which must have previously been created with
- * registered with rtnetlink_link_notifier_register(). */
+ * rtnetlink_link_notifier_register(). */
 void
-rtnetlink_link_notifier_unregister(struct nln_notifier *notifier)
+rtnetlink_link_notifier_destroy(struct nln_notifier *notifier)
 {
-    nln_notifier_unregister(nln, notifier);
+    nln_notifier_destroy(notifier);
 }
 
 /* Calls all of the registered notifiers, passing along any as-yet-unreported

lib/rtnetlink-link.h

@@ -50,9 +50,9 @@ void rtnetlink_link_notify_func(const struct rtnetlink_link_change *change,
 
 bool rtnetlink_link_parse(struct ofpbuf *buf,
                           struct rtnetlink_link_change *change);
-int rtnetlink_link_notifier_register(struct nln_notifier*,
+struct nln_notifier *
-                                     rtnetlink_link_notify_func *, void *aux);
+rtnetlink_link_notifier_create(rtnetlink_link_notify_func *, void *aux);
-void rtnetlink_link_notifier_unregister(struct nln_notifier *);
+void rtnetlink_link_notifier_destroy(struct nln_notifier *);
 void rtnetlink_link_run(void);
 void rtnetlink_link_wait(void);
 #endif /* rtnetlink-link.h */

vswitchd/ovs-brcompatd.c

@@ -799,7 +799,7 @@ int
 main(int argc, char *argv[])
 {
     extern struct vlog_module VLM_reconnect;
-    struct nln_notifier link_notifier;
+    struct nln_notifier *link_notifier;
     struct unixctl_server *unixctl;
     int retval;
 
@@ -823,8 +823,7 @@ main(int argc, char *argv[])
                    "\"brcompat\" kernel module.");
     }
 
-
+    link_notifier = rtnetlink_link_notifier_create(netdev_changed_cb, NULL);
-    rtnetlink_link_notifier_register(&link_notifier, netdev_changed_cb, NULL);
 
     daemonize_complete();
 
@@ -842,7 +841,7 @@ main(int argc, char *argv[])
         poll_block();
     }
 
-    rtnetlink_link_notifier_unregister(&link_notifier);
+    rtnetlink_link_notifier_destroy(link_notifier);
 
     return 0;
 }