From owner-svn-src-all@FreeBSD.ORG Wed Dec 25 03:24:21 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C41297B6; Wed, 25 Dec 2013 03:24:21 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id AF2371AB8; Wed, 25 Dec 2013 03:24:21 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.7/8.14.7) with ESMTP id rBP3OL3S015387; Wed, 25 Dec 2013 03:24:21 GMT (envelope-from glebius@svn.freebsd.org) Received: (from glebius@localhost) by svn.freebsd.org (8.14.7/8.14.7/Submit) id rBP3OKqV015383; Wed, 25 Dec 2013 03:24:20 GMT (envelope-from glebius@svn.freebsd.org) Message-Id: <201312250324.rBP3OKqV015383@svn.freebsd.org> From: Gleb Smirnoff Date: Wed, 25 Dec 2013 03:24:20 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r259859 - head/sys/netinet/libalias X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Dec 2013 03:24:21 -0000 Author: glebius Date: Wed Dec 25 03:24:20 2013 New Revision: 259859 URL: http://svnweb.freebsd.org/changeset/base/259859 Log: 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 [1] Sponsored by: Nginx, Inc. Modified: head/sys/netinet/libalias/alias_db.c head/sys/netinet/libalias/alias_mod.c head/sys/netinet/libalias/alias_mod.h head/sys/netinet/libalias/libalias.3 Modified: head/sys/netinet/libalias/alias_db.c ============================================================================== --- head/sys/netinet/libalias/alias_db.c Wed Dec 25 02:06:57 2013 (r259858) +++ head/sys/netinet/libalias/alias_db.c Wed Dec 25 03:24:20 2013 (r259859) @@ -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 = { Modified: head/sys/netinet/libalias/alias_mod.c ============================================================================== --- head/sys/netinet/libalias/alias_mod.c Wed Dec 25 02:06:57 2013 (r259858) +++ head/sys/netinet/libalias/alias_mod.c Wed Dec 25 03:24:20 2013 (r259859) @@ -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) { Modified: head/sys/netinet/libalias/alias_mod.h ============================================================================== --- head/sys/netinet/libalias/alias_mod.h Wed Dec 25 02:06:57 2013 (r259858) +++ head/sys/netinet/libalias/alias_mod.h Wed Dec 25 03:24:20 2013 (r259859) @@ -45,14 +45,15 @@ MALLOC_DECLARE(M_ALIAS); #endif #endif -/* Packet flow direction. */ -#define IN 1 -#define OUT 2 - -/* Working protocol. */ -#define IP 1 -#define TCP 2 -#define UDP 4 +/* Packet flow direction flags. */ +#define IN 0x0001 +#define OUT 0x0002 +#define NODIR 0x4000 + +/* 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); Modified: head/sys/netinet/libalias/libalias.3 ============================================================================== --- head/sys/netinet/libalias/libalias.3 Wed Dec 25 02:06:57 2013 (r259858) +++ head/sys/netinet/libalias/libalias.3 Wed Dec 25 03:24:20 2013 (r259859) @@ -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 modul 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 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 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