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 <uwe NetBSD.org> [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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
