From 2ee6545f2bff7eb27e8c84965e3ff38dfa909bf6 Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Thu, 15 Sep 2011 11:21:23 -0700 Subject: [PATCH] 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 | 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 ee487951..549c7e44 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 cc930e1d..6541b1e9 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 { @@ -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, - 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++; @@ -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); diff --git a/lib/netlink-notifier.c b/lib/netlink-notifier.c index d0a031b2..94464881 100644 --- a/lib/netlink-notifier.c +++ b/lib/netlink-notifier.c @@ -18,6 +18,7 @@ #include "netlink-notifier.h" +#include #include #include #include @@ -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 e3dacef0..26aa671e 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 c8245dfd..a0c81210 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 eef07d69..0a40ceaf 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 80fbd5cf..23f1ef27 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 df2b75da..3cb1cfc6 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; } -- 2.30.2