Add the infrastructure to support loading multiple versions of TCP

stack modules.

It adds support for mangling symbols exported by a module by prepending
a string to them. (This avoids overlapping symbols in the kernel linker.)

It allows the use of a macro as the module name in the DECLARE_MACRO()
and MACRO_VERSION() macros.

It allows the code to register stack aliases (e.g. both a generic name
["default"] and version-specific name ["default_10_3p1"]).

With these changes, it is trivial to compile TCP stack modules with
the name defined in the Makefile and to load multiple versions of the
same stack simultaneously. This functionality can be used to enable
side-by-side testing of an old and new version of the same TCP stack.
It also could support upgrading the TCP stack without a reboot.

Reviewed by:	gnn, sjg (makefiles only)
Sponsored by:	Netflix
Differential Revision:	https://reviews.freebsd.org/D11086

share/man/man9/Makefile

@@ -1861,6 +1861,8 @@ MLINKS+=taskqueue.9 TASK_INIT.9 \
 	taskqueue.9 taskqueue_unblock.9 \
 	taskqueue.9 TIMEOUT_TASK_INIT.9
 MLINKS+=tcp_functions.9 register_tcp_functions.9 \
+	tcp_functions.9 register_tcp_functions_as_name.9 \
+	tcp_functions.9 register_tcp_functions_as_names.9 \
 	tcp_functions.9 deregister_tcp_functions.9
 MLINKS+=time.9 boottime.9 \
 	time.9 time_second.9 \

share/man/man9/tcp_functions.9

@@ -25,7 +25,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd June 28, 2016
+.Dd March 10, 2017
 .Dt TCP_FUNCTIONS 9
 .Os
 .Sh NAME
@@ -37,6 +37,11 @@
 .Ft int
 .Fn register_tcp_functions "struct tcp_function_block *blk" "int wait"
 .Ft int
+.Fn register_tcp_functions_as_name "struct tcp_function_block *blk" \
+"const char *name" "int wait"
+.Fn register_tcp_functions_as_names "struct tcp_function_block *blk" \
+"int wait" "const char *names[]" "int *num_names"
+.Ft int
 .Fn deregister_tcp_functions "struct tcp_function_block *blk"
 .Sh DESCRIPTION
 The
@@ -62,27 +67,79 @@ This is intentional.
 A "TCP stack" is a collection of functions that implement a set of behavior.
 Therefore, an alternate "function block" defines an alternate "TCP stack".
 .Pp
-.Nm
-modules must call the
-.Fn register_tcp_functions
-function during initialization and successfully call the
-.Fn deregister_tcp_functions
-function prior to allowing the module to be unloaded.
+The
+.Fn register_tcp_functions ,
+.Fn register_tcp_functions_as_name ,
+and
+.Fn register_tcp_functions_as_names
+functions request that the system add a specified function block
+and register it for use with a given name.
+Modules may register the same function block multiple times with different
+names.
+However, names must be globally unique among all registered function blocks.
+Also, modules may not ever modify the contents of the function block (including
+the name) after it has been registered, unless the module first successfully
+de-registers the function block.
 .Pp
 The
 .Fn register_tcp_functions
-function requests that the system add a specified function block to the system.
+function requests that the system register the function block with the name
+defined in the function block's
+.Va tfb_tcp_block_name
+field.
+Note that this is the only one of the three registration functions that
+automatically registers the function block using the name defined in the
+function block's
+.Va tfb_tcp_block_name
+field.
+If a module uses one of the other registration functions, it may request that
+the system register the function block using the name defined in the
+function block's
+.Va tfb_tcp_block_name
+field by explicitly providing that name.
+.Pp
+The
+.Fn register_tcp_functions_as_name
+function requests that the system register the function block with the name
+provided in the
+.Fa name
+argument.
+.Pp
+The
+.Fn register_tcp_functions_as_names
+function requests that the system register the function block with all the
+names provided in the
+.Fa names
+argument.
+The
+.Fa num_names
+argument provides a pointer to the number of names.
+This function will either succeed in registering all of the names in the array,
+or none of the names in the array.
+On failure, the
+.Fa num_names
+argument is updated with the index number of the entry in the
+.Fa names
+array which the system was processing when it encountered the error.
 .Pp
 The
 .Fn deregister_tcp_functions
 function requests that the system remove a specified function block from the
 system.
+If this call succeeds, it will completely deregister the function block,
+regardless of the number of names used to register the function block.
 If the call fails because sockets are still using the specified function block,
 the system will mark the function block as being in the process of being
 removed.
 This will prevent additional sockets from using the specified function block.
 However, it will not impact sockets that are already using the function block.
 .Pp
+.Nm
+modules must call one or more of the registration functions during
+initialization and successfully call the
+.Fn deregister_tcp_functions
+function prior to allowing the module to be unloaded.
+.Pp
 The
 .Fa blk
 argument is a pointer to a
@@ -270,7 +327,9 @@ rest of the system in a standardized format, alternate TCP stacks must
 update all fields in the TCP control block to the greatest extent practical.
 .Sh RETURN VALUES
 The
-.Fn register_tcp_functions
+.Fn register_tcp_functions ,
+.Fn register_tcp_functions_as_name ,
+.Fn register_tcp_functions_as_names ,
 and
 .Fn deregister_tcp_functions
 functions return zero on success and non-zero on failure.

sys/conf/kmod.mk

@@ -253,6 +253,10 @@ ${FULLPROG}: ${OBJS}
 	    export_syms | xargs -J% ${OBJCOPY} % ${.TARGET}
 .endif
 .endif
+.if defined(PREFIX_SYMS)
+	${AWK} -v prefix=${PREFIX_SYMS} -f ${SYSDIR}/conf/kmod_syms_prefix.awk \
+	    ${.TARGET} /dev/null | xargs -J% ${OBJCOPY} % ${.TARGET}
+.endif
 .if !defined(DEBUG_FLAGS) && ${__KLD_SHARED} == no
 	${OBJCOPY} --strip-debug ${.TARGET}
 .endif

sys/conf/kmod_syms_prefix.awk

@@ -0,0 +1,18 @@
+# $FreeBSD$
+
+# Read global symbols from object file.
+BEGIN {
+        while ("${NM:='nm'} " ARGV[1] | getline) {
+                if (match($0, /^[^[:space:]]+ [^AU] (.*)$/)) {
+                        syms[$3] = $2
+                }
+        }
+        delete ARGV[1]
+}
+
+# Strip commons, make everything else local.
+END {
+        for (member in syms) {
+                printf("--redefine-sym=%s=%s%s\n", member, prefix, member);
+        }
+}

sys/netinet/tcp_subr.c

@@ -275,7 +275,7 @@ find_tcp_functions_locked(struct tcp_function_set *fs)
 	struct tcp_function_block *blk=NULL;
 
 	TAILQ_FOREACH(f, &t_functions, tf_next) {
-		if (strcmp(f->tf_fb->tfb_tcp_block_name, fs->function_set_name) == 0) {
+		if (strcmp(f->tf_name, fs->function_set_name) == 0) {
 			blk = f->tf_fb;
 			break;
 		}
@@ -376,6 +376,7 @@ sysctl_net_inet_list_available(SYSCTL_HANDLER_ARGS)
 	struct tcp_function *f;
 	char *buffer, *cp;
 	size_t bufsz, outsz;
+	bool alias;
 
 	cnt = 0;
 	rw_rlock(&tcp_function_lock);
@@ -384,22 +385,25 @@ sysctl_net_inet_list_available(SYSCTL_HANDLER_ARGS)
 	}
 	rw_runlock(&tcp_function_lock);
 
-	bufsz = (cnt+2) * (TCP_FUNCTION_NAME_LEN_MAX + 12) + 1;
+	bufsz = (cnt+2) * ((TCP_FUNCTION_NAME_LEN_MAX * 2) + 13) + 1;
 	buffer = malloc(bufsz, M_TEMP, M_WAITOK);
 
 	error = 0;
 	cp = buffer;
 
-	linesz = snprintf(cp, bufsz, "\n%-32s%c %s\n", "Stack", 'D', "PCB count");
+	linesz = snprintf(cp, bufsz, "\n%-32s%c %-32s %s\n", "Stack", 'D',
+	    "Alias", "PCB count");
 	cp += linesz;
 	bufsz -= linesz;
 	outsz = linesz;
 
 	rw_rlock(&tcp_function_lock);	
 	TAILQ_FOREACH(f, &t_functions, tf_next) {
-		linesz = snprintf(cp, bufsz, "%-32s%c %u\n",
+		alias = (f->tf_name != f->tf_fb->tfb_tcp_block_name);
+		linesz = snprintf(cp, bufsz, "%-32s%c %-32s %u\n",
 		    f->tf_fb->tfb_tcp_block_name,
 		    (f->tf_fb == tcp_func_set_ptr) ? '*' : ' ',
+		    alias ? f->tf_name : "-",
 		    f->tf_fb->tfb_refcnt);
 		if (linesz >= bufsz) {
 			error = EOVERFLOW;
@@ -500,12 +504,31 @@ maketcp_hashsize(int size)
 	return (hashsize);
 }
 
+/*
+ * Register a TCP function block with the name provided in the names
+ * array.  (Note that this function does NOT automatically register
+ * blk->tfb_tcp_block_name as a stack name.  Therefore, you should
+ * explicitly include blk->tfb_tcp_block_name in the list of names if
+ * you wish to register the stack with that name.)
+ *
+ * Either all name registrations will succeed or all will fail.  If
+ * a name registration fails, the function will update the num_names
+ * argument to point to the array index of the name that encountered
+ * the failure.
+ *
+ * Returns 0 on success, or an error code on failure.
+ */
 int
-register_tcp_functions(struct tcp_function_block *blk, int wait)
+register_tcp_functions_as_names(struct tcp_function_block *blk, int wait,
+    const char *names[], int *num_names)
 {
-	struct tcp_function_block *lblk;
 	struct tcp_function *n;
 	struct tcp_function_set fs;
+	int error, i;
+
+	KASSERT(names != NULL && *num_names > 0,
+	    ("%s: Called with 0-length name list", __func__));
+	KASSERT(names != NULL, ("%s: Called with NULL name list", __func__));
 
 	if (t_functions_inited == 0) {
 		init_tcp_functions();
@@ -518,6 +541,7 @@ register_tcp_functions(struct tcp_function_block *blk, int wait)
 		 * These functions are required and you
 		 * need a name.
 		 */
+		*num_names = 0;
 		return (EINVAL);
 	}
 	if (blk->tfb_tcp_timer_stop_all ||
@@ -532,34 +556,99 @@ register_tcp_functions(struct tcp_function_block *blk, int wait)
 		    (blk->tfb_tcp_timer_activate == NULL) ||
 		    (blk->tfb_tcp_timer_active == NULL) ||
 		    (blk->tfb_tcp_timer_stop == NULL)) {
-			return (EINVAL);			
+			*num_names = 0;
+			return (EINVAL);
 		}
-	}	
-	n = malloc(sizeof(struct tcp_function), M_TCPFUNCTIONS, wait);
-	if (n == NULL) {
-		return (ENOMEM);
-	}
-	n->tf_fb = blk;
-	strcpy(fs.function_set_name, blk->tfb_tcp_block_name);
-	rw_wlock(&tcp_function_lock);
-	lblk = find_tcp_functions_locked(&fs);
-	if (lblk) {
-		/* Duplicate name space not allowed */
-		rw_wunlock(&tcp_function_lock);
-		free(n, M_TCPFUNCTIONS);
-		return (EALREADY);
 	}
+
 	refcount_init(&blk->tfb_refcnt, 0);
 	blk->tfb_flags = 0;
-	TAILQ_INSERT_TAIL(&t_functions, n, tf_next);
-	rw_wunlock(&tcp_function_lock);
+	for (i = 0; i < *num_names; i++) {
+		n = malloc(sizeof(struct tcp_function), M_TCPFUNCTIONS, wait);
+		if (n == NULL) {
+			error = ENOMEM;
+			goto cleanup;
+		}
+		n->tf_fb = blk;
+
+		(void)strncpy(fs.function_set_name, names[i],
+		    TCP_FUNCTION_NAME_LEN_MAX);
+		fs.function_set_name[TCP_FUNCTION_NAME_LEN_MAX - 1] = '\0';
+		rw_wlock(&tcp_function_lock);
+		if (find_tcp_functions_locked(&fs) != NULL) {
+			/* Duplicate name space not allowed */
+			rw_wunlock(&tcp_function_lock);
+			free(n, M_TCPFUNCTIONS);
+			error = EALREADY;
+			goto cleanup;
+		}
+		(void)strncpy(n->tf_name, names[i], TCP_FUNCTION_NAME_LEN_MAX);
+		n->tf_name[TCP_FUNCTION_NAME_LEN_MAX - 1] = '\0';
+		TAILQ_INSERT_TAIL(&t_functions, n, tf_next);
+		rw_wunlock(&tcp_function_lock);
+	}
 	return(0);
-}	
+
+cleanup:
+	/*
+	 * Deregister the names we just added. Because registration failed
+	 * for names[i], we don't need to deregister that name.
+	 */
+	*num_names = i;
+	rw_wlock(&tcp_function_lock);
+	while (--i >= 0) {
+		TAILQ_FOREACH(n, &t_functions, tf_next) {
+			if (!strncmp(n->tf_name, names[i],
+			    TCP_FUNCTION_NAME_LEN_MAX)) {
+				TAILQ_REMOVE(&t_functions, n, tf_next);
+				n->tf_fb = NULL;
+				free(n, M_TCPFUNCTIONS);
+				break;
+			}
+		}
+	}
+	rw_wunlock(&tcp_function_lock);
+	return (error);
+}
+
+/*
+ * Register a TCP function block using the name provided in the name
+ * argument.
+ *
+ * Returns 0 on success, or an error code on failure.
+ */
+int
+register_tcp_functions_as_name(struct tcp_function_block *blk, const char *name,
+    int wait)
+{
+	const char *name_list[1];
+	int num_names, rv;
+
+	num_names = 1;
+	if (name != NULL)
+		name_list[0] = name;
+	else
+		name_list[0] = blk->tfb_tcp_block_name;
+	rv = register_tcp_functions_as_names(blk, wait, name_list, &num_names);
+	return (rv);
+}
+
+/*
+ * Register a TCP function block using the name defined in
+ * blk->tfb_tcp_block_name.
+ *
+ * Returns 0 on success, or an error code on failure.
+ */
+int
+register_tcp_functions(struct tcp_function_block *blk, int wait)
+{
+
+	return (register_tcp_functions_as_name(blk, NULL, wait));
+}
 
 int
 deregister_tcp_functions(struct tcp_function_block *blk)
 {
-	struct tcp_function_block *lblk;
 	struct tcp_function *f;
 	int error=ENOENT;
 	
@@ -579,8 +668,7 @@ deregister_tcp_functions(struct tcp_function_block *blk)
 		rw_wunlock(&tcp_function_lock);		
 		return (EBUSY);
 	}
-	lblk = find_tcp_fb_locked(blk, &f);
-	if (lblk) {
+	while (find_tcp_fb_locked(blk, &f) != NULL) {
 		/* Found */
 		TAILQ_REMOVE(&t_functions, f, tf_next);
 		f->tf_fb = NULL;

sys/netinet/tcp_var.h

@@ -272,8 +272,9 @@ struct tcp_function_block {
 };
 
 struct tcp_function {
-	TAILQ_ENTRY(tcp_function) tf_next;
-	struct tcp_function_block *tf_fb;
+	TAILQ_ENTRY(tcp_function)	tf_next;
+	char				tf_name[TCP_FUNCTION_NAME_LEN_MAX];
+	struct tcp_function_block	*tf_fb;
 };
 
 TAILQ_HEAD(tcp_funchead, tcp_function);
@@ -785,6 +786,10 @@ void	 tcp_do_segment(struct mbuf *, struct tcphdr *,
 			int);
 
 int register_tcp_functions(struct tcp_function_block *blk, int wait);
+int register_tcp_functions_as_names(struct tcp_function_block *blk,
+    int wait, const char *names[], int *num_names);
+int register_tcp_functions_as_name(struct tcp_function_block *blk,
+    const char *name, int wait);
 int deregister_tcp_functions(struct tcp_function_block *blk);
 struct tcp_function_block *find_and_ref_tcp_functions(struct tcp_function_set *fs);
 struct tcp_function_block *find_and_ref_tcp_fb(struct tcp_function_block *blk);

sys/sys/module.h

@@ -106,14 +106,15 @@ struct mod_pnp_match_info
 
 #include <sys/linker_set.h>
 
+#define	MODULE_METADATA_CONCAT(uniquifier)	_mod_metadata##uniquifier
 #define	MODULE_METADATA(uniquifier, type, data, cval)			\
-	static struct mod_metadata _mod_metadata##uniquifier = {	\
+	static struct mod_metadata MODULE_METADATA_CONCAT(uniquifier) = {	\
 		MDT_STRUCT_VERSION,					\
 		type,							\
 		data,							\
 		cval							\
 	};								\
-	DATA_SET(modmetadata_set, _mod_metadata##uniquifier)
+	DATA_SET(modmetadata_set, MODULE_METADATA_CONCAT(uniquifier))
 
 #define	MODULE_DEPEND(module, mdepend, vmin, vpref, vmax)		\
 	static struct mod_depend _##module##_depend_on_##mdepend	\
@@ -139,7 +140,7 @@ struct mod_pnp_match_info
 #define	DECLARE_MODULE_WITH_MAXVER(name, data, sub, order, maxver)	\
 	MODULE_DEPEND(name, kernel, __FreeBSD_version,			\
 	    __FreeBSD_version, maxver);			\
-	MODULE_METADATA(_md_##name, MDT_MODULE, &data, #name);		\
+	MODULE_METADATA(_md_##name, MDT_MODULE, &data, __XSTRING(name));\
 	SYSINIT(name##module, sub, order, module_register_init, &data);	\
 	struct __hack
 
@@ -156,13 +157,14 @@ struct mod_pnp_match_info
 #define	DECLARE_MODULE_TIED(name, data, sub, order)				\
 	DECLARE_MODULE_WITH_MAXVER(name, data, sub, order, __FreeBSD_version)
 
+#define	MODULE_VERSION_CONCAT(module, version)	_##module##_version
 #define	MODULE_VERSION(module, version)					\
-	static struct mod_version _##module##_version			\
+	static struct mod_version MODULE_VERSION_CONCAT(module, version)\
 	    __section(".data") = {					\
 		version							\
 	};								\
-	MODULE_METADATA(_##module##_version, MDT_VERSION,		\
-	    &_##module##_version, #module)
+	MODULE_METADATA(MODULE_VERSION_CONCAT(module, version), MDT_VERSION,\
+	    &MODULE_VERSION_CONCAT(module, version), __XSTRING(module))
 
 /**
  * Generic macros to create pnp info hints that modules may export