[ovs-dev] [notifier 2/2] notifiers: Create and destroy nln_notifiers.
Ethan Jackson
ethan at nicira.com
Thu Sep 15 23:06:22 UTC 2011
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 | 16 +++++---------
lib/netdev-linux.c | 18 ++++++++++------
lib/netlink-notifier.c | 49 ++++++++++++++++++++++++++++++++-------------
lib/netlink-notifier.h | 14 ++++--------
lib/route-table.c | 16 +++++++++-----
lib/rtnetlink-link.c | 17 +++++++--------
lib/rtnetlink-link.h | 6 ++--
vswitchd/ovs-brcompatd.c | 7 ++---
8 files changed, 81 insertions(+), 62 deletions(-)
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 16e5dbf..87d3433 100644
--- a/lib/dpif-linux.c
+++ b/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_linux_port_changed, dpif);
- if (error) {
+ dpif->port_notifier = nln_notifier_create(nln, dpif_linux_port_changed,
+ dpif);
+ 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_unregister(nln, &dpif->port_notifier);
- }
-
+ nln_notifier_destroy(dpif->port_notifier);
nl_sock_destroy(dpif->mc_sock);
sset_destroy(&dpif->changed_ports);
free(dpif->lru_bitmap);
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index fbc0a67..5c2dfd8 100644
--- a/lib/netdev-linux.c
+++ b/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 {
@@ -529,13 +529,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,
- netdev_linux_cache_cb, NULL);
- if (error) {
- return error;
+ assert(!netdev_linux_cache_notifier);
+
+ netdev_linux_cache_notifier =
+ rtnetlink_link_notifier_create(netdev_linux_cache_cb, NULL);
+
+ if (!netdev_linux_cache_notifier) {
+ return EINVAL;
}
}
cache_notifier_refcount++;
@@ -625,7 +627,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);
diff --git a/lib/netlink-notifier.c b/lib/netlink-notifier.c
index d0a031b..9446488 100644
--- a/lib/netlink-notifier.c
+++ b/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. */
-int
-nln_notifier_register(struct nln *nln, struct nln_notifier *notifier,
- nln_notify_func *cb, void *aux)
+ * Returns an initialized nln_notifier if successful, otherwise NULL. */
+struct nln_notifier *
+nln_notifier_create(struct nln *nln, 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,21 +126,28 @@ nln_notifier_register(struct nln *nln, struct nln_notifier *notifier,
nln_run(nln);
}
+ notifier = xmalloc(sizeof *notifier);
list_push_back(&nln->all_notifiers, ¬ifier->node);
notifier->cb = cb;
notifier->aux = aux;
- return 0;
+ notifier->nln = nln;
+ return notifier;
}
-/* Cancels notification on 'notifier', which must have previously been
- * registered with nln_notifier_register(). */
+/* Destroys 'notifier', which must have previously been created with
+ * nln_notifier_register(). */
void
-nln_notifier_unregister(struct nln *nln, struct nln_notifier *notifier)
+nln_notifier_destroy(struct nln_notifier *notifier)
{
- list_remove(¬ifier->node);
- if (list_is_empty(&nln->all_notifiers)) {
- nl_sock_destroy(nln->notify_sock);
- nln->notify_sock = NULL;
+ if (notifier) {
+ struct nln *nln = notifier->nln;
+
+ list_remove(¬ifier->node);
+ if (list_is_empty(&nln->all_notifiers)) {
+ nl_sock_destroy(nln->notify_sock);
+ nln->notify_sock = NULL;
+ }
+ free(notifier);
}
}
diff --git a/lib/netlink-notifier.h b/lib/netlink-notifier.h
index e3dacef..26aa671 100644
--- a/lib/netlink-notifier.h
+++ b/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 *,
- nln_notify_func *, void *aux);
-void nln_notifier_unregister(struct nln *, struct nln_notifier *);
+struct nln_notifier *nln_notifier_create(struct nln *, nln_notify_func *,
+ void *aux);
+void nln_notifier_destroy(struct nln_notifier *);
void nln_run(struct nln *);
void nln_wait(struct nln *);
#endif /* netlink-notifier.h */
diff --git a/lib/route-table.c b/lib/route-table.c
index c8245df..a0c8121 100644
--- a/lib/route-table.c
+++ b/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 name_notifier;
+static struct nln_notifier *route_notifier = NULL;
+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);
}
diff --git a/lib/rtnetlink-link.c b/lib/rtnetlink-link.c
index eef07d6..0a40cea 100644
--- a/lib/rtnetlink-link.c
+++ b/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. */
-int
-rtnetlink_link_notifier_register(struct nln_notifier *notifier,
- rtnetlink_link_notify_func *cb, void *aux)
+ * Returns an initialized nln_notifier if successful, NULL otherwise. */
+struct nln_notifier *
+rtnetlink_link_notifier_create(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
- * registered with rtnetlink_link_notifier_register(). */
+/* Destroys 'notifier', which must have previously been created with
+ * 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
diff --git a/lib/rtnetlink-link.h b/lib/rtnetlink-link.h
index 80fbd5c..23f1ef2 100644
--- a/lib/rtnetlink-link.h
+++ b/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*,
- rtnetlink_link_notify_func *, void *aux);
-void rtnetlink_link_notifier_unregister(struct nln_notifier *);
+struct nln_notifier *
+rtnetlink_link_notifier_create(rtnetlink_link_notify_func *, void *aux);
+void rtnetlink_link_notifier_destroy(struct nln_notifier *);
void rtnetlink_link_run(void);
void rtnetlink_link_wait(void);
#endif /* rtnetlink-link.h */
diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c
index 7d03472..69143ec 100644
--- a/vswitchd/ovs-brcompatd.c
+++ b/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.");
}
-
- rtnetlink_link_notifier_register(&link_notifier, netdev_changed_cb, NULL);
+ link_notifier = rtnetlink_link_notifier_create(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;
}
--
1.7.6.1
More information about the dev
mailing list