diff options
author | glebius <glebius@FreeBSD.org> | 2013-12-25 03:24:20 +0000 |
---|---|---|
committer | glebius <glebius@FreeBSD.org> | 2013-12-25 03:24:20 +0000 |
commit | 4c8ca3b238b352dfeb70f59aba3f784e9a1fea12 (patch) | |
tree | d938b565d5e6a06697b38b4db981b88bc649a8d1 | |
parent | dc2a6c5e7bf747f0362c04fe0cbce417d7369bde (diff) | |
download | FreeBSD-src-4c8ca3b238b352dfeb70f59aba3f784e9a1fea12.zip FreeBSD-src-4c8ca3b238b352dfeb70f59aba3f784e9a1fea12.tar.gz |
Cleanup alias module handler register/unregister.
- Remove locking, since all module(9) events are running under &Giant.
- Use TAILQ for protocol handlers and fix a bug which led to
infinite cycle. Bug found in VirtualBox [1]
- Simplify code everywhere.
- Fix documentation.
[1] https://www.virtualbox.org/pipermail/vbox-dev/2013-November/011936.html
PR: 183792 [1]
Submitted by: Valery Ushakov <uwe NetBSD.org> [1]
Sponsored by: Nginx, Inc.
-rw-r--r-- | sys/netinet/libalias/alias_db.c | 14 | ||||
-rw-r--r-- | sys/netinet/libalias/alias_mod.c | 183 | ||||
-rw-r--r-- | sys/netinet/libalias/alias_mod.h | 26 | ||||
-rw-r--r-- | sys/netinet/libalias/libalias.3 | 26 |
4 files changed, 63 insertions, 186 deletions
diff --git a/sys/netinet/libalias/alias_db.c b/sys/netinet/libalias/alias_db.c index 7df55d0..cceccd5 100644 --- a/sys/netinet/libalias/alias_db.c +++ b/sys/netinet/libalias/alias_db.c @@ -349,24 +349,16 @@ MODULE_VERSION(libalias, 1); static int alias_mod_handler(module_t mod, int type, void *data) { - int error; switch (type) { - case MOD_LOAD: - error = 0; - handler_chain_init(); - break; case MOD_QUIESCE: case MOD_UNLOAD: - handler_chain_destroy(); finishoff(); - error = 0; - break; + case MOD_LOAD: + return (0); default: - error = EINVAL; + return (EINVAL); } - - return (error); } static moduledata_t alias_mod = { diff --git a/sys/netinet/libalias/alias_mod.c b/sys/netinet/libalias/alias_mod.c index a061431..6ac4424 100644 --- a/sys/netinet/libalias/alias_mod.c +++ b/sys/netinet/libalias/alias_mod.c @@ -52,201 +52,82 @@ __FBSDID("$FreeBSD$"); #endif /* Protocol and userland module handlers chains. */ -LIST_HEAD(handler_chain, proto_handler) handler_chain = LIST_HEAD_INITIALIZER(handler_chain); -#ifdef _KERNEL -struct rwlock handler_rw; -#endif -SLIST_HEAD(dll_chain, dll) dll_chain = SLIST_HEAD_INITIALIZER(dll_chain); - -#ifdef _KERNEL - -#define LIBALIAS_RWLOCK_INIT() \ - rw_init(&handler_rw, "Libalias_modules_rwlock") -#define LIBALIAS_RWLOCK_DESTROY() rw_destroy(&handler_rw) -#define LIBALIAS_WLOCK_ASSERT() \ - rw_assert(&handler_rw, RA_WLOCKED) - -static __inline void -LIBALIAS_RLOCK(void) -{ - rw_rlock(&handler_rw); -} - -static __inline void -LIBALIAS_RUNLOCK(void) -{ - rw_runlock(&handler_rw); -} - -static __inline void -LIBALIAS_WLOCK(void) -{ - rw_wlock(&handler_rw); -} - -static __inline void -LIBALIAS_WUNLOCK(void) -{ - rw_wunlock(&handler_rw); -} - -static void -_handler_chain_init(void) -{ - - if (!rw_initialized(&handler_rw)) - LIBALIAS_RWLOCK_INIT(); -} - -static void -_handler_chain_destroy(void) -{ - - if (rw_initialized(&handler_rw)) - LIBALIAS_RWLOCK_DESTROY(); -} - -#else -#define LIBALIAS_RWLOCK_INIT() ; -#define LIBALIAS_RWLOCK_DESTROY() ; -#define LIBALIAS_WLOCK_ASSERT() ; -#define LIBALIAS_RLOCK() ; -#define LIBALIAS_RUNLOCK() ; -#define LIBALIAS_WLOCK() ; -#define LIBALIAS_WUNLOCK() ; -#define _handler_chain_init() ; -#define _handler_chain_destroy() ; -#endif - -void -handler_chain_init(void) -{ - _handler_chain_init(); -} - -void -handler_chain_destroy(void) -{ - _handler_chain_destroy(); -} +static TAILQ_HEAD(handler_chain, proto_handler) handler_chain = + TAILQ_HEAD_INITIALIZER(handler_chain); static int -_attach_handler(struct proto_handler *p) +attach_handler(struct proto_handler *p) { struct proto_handler *b; - LIBALIAS_WLOCK_ASSERT(); - b = NULL; - LIST_FOREACH(b, &handler_chain, entries) { + TAILQ_FOREACH(b, &handler_chain, link) { if ((b->pri == p->pri) && (b->dir == p->dir) && (b->proto == p->proto)) - return (EEXIST); /* Priority conflict. */ + return (EEXIST); if (b->pri > p->pri) { - LIST_INSERT_BEFORE(b, p, entries); + TAILQ_INSERT_BEFORE(b, p, link); return (0); } } - /* End of list or found right position, inserts here. */ - if (b) - LIST_INSERT_AFTER(b, p, entries); - else - LIST_INSERT_HEAD(&handler_chain, p, entries); - return (0); -} -static int -_detach_handler(struct proto_handler *p) -{ - struct proto_handler *b, *b_tmp; + TAILQ_INSERT_TAIL(&handler_chain, p, link); - LIBALIAS_WLOCK_ASSERT(); - LIST_FOREACH_SAFE(b, &handler_chain, entries, b_tmp) { - if (b == p) { - LIST_REMOVE(b, entries); - return (0); - } - } - return (ENOENT); /* Handler not found. */ + return (0); } int -LibAliasAttachHandlers(struct proto_handler *_p) +LibAliasAttachHandlers(struct proto_handler *p) { - int i, error; + int error; - LIBALIAS_WLOCK(); - error = -1; - for (i = 0; 1; i++) { - if (*((int *)&_p[i]) == EOH) - break; - error = _attach_handler(&_p[i]); - if (error != 0) - break; + while (p->dir != NODIR) { + error = attach_handler(p); + if (error) + return (error); + p++; } - LIBALIAS_WUNLOCK(); - return (error); + + return (0); } +/* XXXGL: should be void, but no good reason to break ABI */ int -LibAliasDetachHandlers(struct proto_handler *_p) +LibAliasDetachHandlers(struct proto_handler *p) { - int i, error; - LIBALIAS_WLOCK(); - error = -1; - for (i = 0; 1; i++) { - if (*((int *)&_p[i]) == EOH) - break; - error = _detach_handler(&_p[i]); - if (error != 0) - break; + while (p->dir != NODIR) { + TAILQ_REMOVE(&handler_chain, p, link); + p++; } - LIBALIAS_WUNLOCK(); - return (error); -} - -int -detach_handler(struct proto_handler *_p) -{ - int error; - LIBALIAS_WLOCK(); - error = -1; - error = _detach_handler(_p); - LIBALIAS_WUNLOCK(); - return (error); + return (0); } int -find_handler(int8_t dir, int8_t proto, struct libalias *la, __unused struct ip *pip, +find_handler(int8_t dir, int8_t proto, struct libalias *la, struct ip *ip, struct alias_data *ad) { struct proto_handler *p; - int error; - LIBALIAS_RLOCK(); - error = ENOENT; - LIST_FOREACH(p, &handler_chain, entries) { - if ((p->dir & dir) && (p->proto & proto)) - if (p->fingerprint(la, ad) == 0) { - error = p->protohandler(la, pip, ad); - break; - } - } - LIBALIAS_RUNLOCK(); - return (error); + TAILQ_FOREACH(p, &handler_chain, link) + if ((p->dir & dir) && (p->proto & proto) && + p->fingerprint(la, ad) == 0) + return (p->protohandler(la, ip, ad)); + + return (ENOENT); } struct proto_handler * first_handler(void) { - return (LIST_FIRST(&handler_chain)); + return (TAILQ_FIRST(&handler_chain)); } #ifndef _KERNEL /* Dll manipulation code - this code is not thread safe... */ +SLIST_HEAD(dll_chain, dll) dll_chain = SLIST_HEAD_INITIALIZER(dll_chain); int attach_dll(struct dll *p) { diff --git a/sys/netinet/libalias/alias_mod.h b/sys/netinet/libalias/alias_mod.h index c24c0bc..12cb1be 100644 --- a/sys/netinet/libalias/alias_mod.h +++ b/sys/netinet/libalias/alias_mod.h @@ -45,14 +45,15 @@ MALLOC_DECLARE(M_ALIAS); #endif #endif -/* Packet flow direction. */ -#define IN 1 -#define OUT 2 +/* Packet flow direction flags. */ +#define IN 0x0001 +#define OUT 0x0002 +#define NODIR 0x4000 -/* Working protocol. */ -#define IP 1 -#define TCP 2 -#define UDP 4 +/* Working protocol flags. */ +#define IP 0x01 +#define TCP 0x02 +#define UDP 0x04 /* * Data passed to protocol handler module, it must be filled @@ -81,18 +82,15 @@ struct proto_handler { /* Aliasing * function. */ int (*protohandler)(struct libalias *, struct ip *, struct alias_data *); - LIST_ENTRY(proto_handler) entries; -} -; + TAILQ_ENTRY(proto_handler) link; +}; + /* End of handlers. */ -#define EOH -1 +#define EOH .dir = NODIR /* Functions used with protocol handlers. */ -void handler_chain_init(void); -void handler_chain_destroy(void); int LibAliasAttachHandlers(struct proto_handler *); int LibAliasDetachHandlers(struct proto_handler *); -int detach_handler(struct proto_handler *); int find_handler(int8_t, int8_t, struct libalias *, struct ip *, struct alias_data *); struct proto_handler *first_handler(void); diff --git a/sys/netinet/libalias/libalias.3 b/sys/netinet/libalias/libalias.3 index 4d02cc4..45e7fa4 100644 --- a/sys/netinet/libalias/libalias.3 +++ b/sys/netinet/libalias/libalias.3 @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd July 4, 2011 +.Dd December 25, 2013 .Dt LIBALIAS 3 .Os .Sh NAME @@ -1103,7 +1103,7 @@ struct proto_handler { struct ip *pip, struct alias_data *ah); int (*protohandler)(struct libalias *la, struct ip *pip, struct alias_data *ah); - LIST_ENTRY(proto_handler) entries; + TAILQ_ENTRY(proto_handler) link; }; .Ed .Pp @@ -1262,8 +1262,16 @@ Here we analyse some parts of that module. From .Pa alias_dummy.c : .Bd -literal -struct proto_handler handlers [] = {{666, IN|OUT, UDP|TCP, - &fingerprint, &protohandler}}; +struct proto_handler handlers[] = { + { + .pri = 666, + .dir = IN|OUT, + .proto = UDP|TCP, + .fingerprint = fingerprint, + .protohandler= protohandler, + }, + { EOH } +}; .Ed .Pp The variable @@ -1299,12 +1307,10 @@ mod_handler(module_t mod, int type, void *data) switch (type) { case MOD_LOAD: - error = 0; - attach_handlers(handlers); + error = LibAliasAttachHandlers(handlers); break; case MOD_UNLOAD: - error = 0; - detach_handlers(handlers; + error = LibAliasDetachHandlers(handlers); break; default: error = EINVAL; @@ -1315,9 +1321,9 @@ mod_handler(module_t mod, int type, void *data) When running as KLD, .Fn mod_handler registers/deregisters the module using -.Fn attach_handlers +.Fn LibAliasAttachHandlers and -.Fn detach_handlers , +.Fn LibAliasDetachHandlers , respectively. .Pp Every module must contain at least 2 functions: one fingerprint |