[PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

There seems to be rough consensus that the kernel currently has too many 
exported symbols. A lot of these exports are generally usable utility 
functions or important driver interfaces; but another large part are functions
intended by only one or two very specific modules for a very specific purpose.
One example is the TCP code. It has most of its internals exported, but 
only for use by tcp_ipv6.c (and now a few more by the TCP/IP congestion modules) 
But it doesn't make sense to include these exported for a specific module
functions into a broader "kernel interface".   External modules assume
they can use these functions, but they were never intended for that.

This patch allows to export symbols only for specific modules by 
introducing symbol name spaces. A module name space has a white
list of modules that are allowed to import symbols for it; all others
can't use the symbols.

It adds two new macros: 

MODULE_NAMESPACE_ALLOW(namespace, module);

Allow module to import symbols from namespace. module is the module name without
.ko as displayed by lsmod.  Must be in the same module as the export
(and be duplicated if there are multiple modules exporting symbols
to a namespace).  Multiple allows for the same name space are allowed.

EXPORT_SYMBOL_NS(namespace, symbol);

Export symbol into namespace.  Only modules allowed for the namespace
will be able to use them. EXPORT_SYMBOL_NS implies GPL only
because it is only for "internal" interfaces.

The name spaces only work for module loading. I didn't find
a nice way to make them work inside the main kernel binary. This means
the name space is not enforced for modules that are built in.

The biggest amount of work is of course still open: to go over all the existing
exports and figure for which ones it makes sense to define a namespace.
I did it for TCP and UDP so far, but the kernel right now has nearly 10k
exports (with some dups) that would need to be checked and turned into
name spaces. I would expect any symbol that is only used by one or two
other modules is a strong candidate for a namespace; in some cases even more
with modules that are tightly coupled.

I am optimistic that in the end we will have a much more manageable 
kernel interface.

Caveats: 

Exports need one long word more memory.

I had to add some alignment magic to the existing EXPORT_SYMBOLs
to get the sections right. Tested on i386/x86-64, but I hope it also
still works on architectures with stricter alignment requirements
like ARM. Any testers for that?

---
 arch/arm/kernel/armksyms.c        |    2 
 include/asm-generic/vmlinux.lds.h |    7 +
 include/linux/module.h            |   71 +++++++++++++++----
 kernel/module.c                   |  137 +++++++++++++++++++++++++++++++-------
 4 files changed, 177 insertions(+), 40 deletions(-)

Index: linux/include/linux/module.h
===================================================================
--- linux.orig/include/linux/module.h
+++ linux/include/linux/module.h
@@ -34,6 +34,7 @@ struct kernel_symbol
 {
 	unsigned long value;
 	const char *name;
+	const char *namespace;
 };
 
 struct modversion_info
@@ -167,49 +168,80 @@ struct notifier_block;
 #ifdef CONFIG_MODULES
 
 /* Get/put a kernel symbol (calls must be symmetric) */
-void *__symbol_get(const char *symbol);
-void *__symbol_get_gpl(const char *symbol);
+extern void *do_symbol_get(const char *symbol, struct module *caller);
+#define __symbol_get(sym) do_symbol_get(sym, THIS_MODULE)
 #define symbol_get(x) ((typeof(&x))(__symbol_get(MODULE_SYMBOL_PREFIX #x)))
 
+struct module_ns {
+	char *name;
+	char *allowed;
+};
+
+#define NS_SEPARATOR "."
+
+/*
+ * Allow module MODULE to reference namespace NS.
+ * MODULE is just the base module name with suffix or path.
+ * This must be declared in the module (or main kernel) as where the
+ * symbols are defined. When multiple modules export symbols from
+ * a single namespace all modules need to contain a full set
+ * of MODULE_NAMESPACE_ALLOWs.
+ */
+#define MODULE_NAMESPACE_ALLOW(ns, module) \
+	static const struct module_ns __knamespace_##module##_##_##ns \
+	asm("__knamespace_" #module NS_SEPARATOR #ns)		\
+	__attribute_used__					\
+	__attribute__((section("__knamespace"), unused))	\
+	= { #ns,  #module }
+
 #ifndef __GENKSYMS__
 #ifdef CONFIG_MODVERSIONS
 /* Mark the CRC weak since genksyms apparently decides not to
  * generate a checksums for some symbols */
-#define __CRC_SYMBOL(sym, sec)					\
+#define __CRC_SYMBOL(sym, sec, post, post2)			\
 	extern void *__crc_##sym __attribute__((weak));		\
-	static const unsigned long __kcrctab_##sym		\
+	static const unsigned long __kcrctab_##sym##post	\
+	asm("__kcrctab_" #sym post2)				\
 	__attribute_used__					\
 	__attribute__((section("__kcrctab" sec), unused))	\
 	= (unsigned long) &__crc_##sym;
 #else
-#define __CRC_SYMBOL(sym, sec)
+#define __CRC_SYMBOL(sym, sec, post, post2)
 #endif
 
 /* For every exported symbol, place a struct in the __ksymtab section */
-#define __EXPORT_SYMBOL(sym, sec)				\
+#define __EXPORT_SYMBOL(sym, sec, post, post2, ns)		\
 	extern typeof(sym) sym;					\
-	__CRC_SYMBOL(sym, sec)					\
-	static const char __kstrtab_##sym[]			\
+	__CRC_SYMBOL(sym, sec, post, post2)			\
+	static const char __kstrtab_##sym##post[]		\
+	asm("__kstrtab_" #sym post2)				\
 	__attribute__((section("__ksymtab_strings")))		\
 	= MODULE_SYMBOL_PREFIX #sym;                    	\
-	static const struct kernel_symbol __ksymtab_##sym	\
+	static const struct kernel_symbol __ksymtab_##sym##post	\
+	asm("__ksymtab_" #sym post2)				\
 	__attribute_used__					\
 	__attribute__((section("__ksymtab" sec), unused))	\
-	= { (unsigned long)&sym, __kstrtab_##sym }
+	__attribute__((aligned(sizeof(void *))))		\
+	= { (unsigned long)&sym, __kstrtab_##sym##post, ns }
 
 #define EXPORT_SYMBOL(sym)					\
-	__EXPORT_SYMBOL(sym, "")
+	__EXPORT_SYMBOL(sym, "",,, NULL)
 
 #define EXPORT_SYMBOL_GPL(sym)					\
-	__EXPORT_SYMBOL(sym, "_gpl")
+	__EXPORT_SYMBOL(sym, "_gpl",,, NULL)
 
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)				\
-	__EXPORT_SYMBOL(sym, "_gpl_future")
+	__EXPORT_SYMBOL(sym, "_gpl_future",,, NULL)
 
+/* Export symbol into namespace ns
+ * No _GPL variants because namespaces imply GPL only
+ */
+#define EXPORT_SYMBOL_NS(ns, sym)				\
+	__EXPORT_SYMBOL(sym, "_gpl",__##ns, NS_SEPARATOR #ns, #ns)
 
 #ifdef CONFIG_UNUSED_SYMBOLS
-#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused")
-#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl")
+#define EXPORT_UNUSED_SYMBOL(sym) __EXPORT_SYMBOL(sym, "_unused",,,NULL)
+#define EXPORT_UNUSED_SYMBOL_GPL(sym) __EXPORT_SYMBOL(sym, "_unused_gpl",,,NULL)
 #else
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
@@ -288,6 +320,10 @@ struct module
 	unsigned int num_gpl_future_syms;
 	const unsigned long *gpl_future_crcs;
 
+	/* Namespaces */
+	struct module_ns *knamespace;
+	unsigned num_knamespaces;
+
 	/* Exception table */
 	unsigned int num_exentries;
 	const struct exception_table_entry *extable;
@@ -391,7 +427,8 @@ extern void __module_put_and_exit(struct
 
 #ifdef CONFIG_MODULE_UNLOAD
 unsigned int module_refcount(struct module *mod);
-void __symbol_put(const char *symbol);
+extern void do_symbol_put(const char *symbol, struct module *caller);
+#define __symbol_put(symbol) do_symbol_put(symbol, THIS_MODULE)
 #define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x)
 void symbol_put_addr(void *addr);
 
@@ -470,6 +507,8 @@ extern void module_update_markers(struct
 #define EXPORT_SYMBOL_GPL_FUTURE(sym)
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
+#define EXPORT_SYMBOL_NS(sym, ns)
+#define MODULE_NAMESPACE_ALLOW(ns, allow)
 
 /* Given an address, look for it in the exception tables. */
 static inline const struct exception_table_entry *
Index: linux/kernel/module.c
===================================================================
--- linux.orig/kernel/module.c
+++ linux/kernel/module.c
@@ -140,6 +140,8 @@ extern const unsigned long __start___kcr
 extern const unsigned long __start___kcrctab_gpl_future[];
 extern const unsigned long __start___kcrctab_unused[];
 extern const unsigned long __start___kcrctab_unused_gpl[];
+extern const struct module_ns __start___knamespace[];
+extern const struct module_ns __stop___knamespace[];
 
 #ifndef CONFIG_MODVERSIONS
 #define symversion(base, idx) NULL
@@ -171,7 +173,7 @@ static void printk_unused_warning(const 
 }
 
 /* Find a symbol, return value, crc and module which owns it */
-static unsigned long __find_symbol(const char *name,
+static const struct kernel_symbol *do_find_symbol(const char *name,
 				   struct module **owner,
 				   const unsigned long **crc,
 				   int gplok)
@@ -184,7 +186,7 @@ static unsigned long __find_symbol(const
 	ks = lookup_symbol(name, __start___ksymtab, __stop___ksymtab);
 	if (ks) {
 		*crc = symversion(__start___kcrctab, (ks - __start___ksymtab));
-		return ks->value;
+		return ks;
 	}
 	if (gplok) {
 		ks = lookup_symbol(name, __start___ksymtab_gpl,
@@ -192,7 +194,7 @@ static unsigned long __find_symbol(const
 		if (ks) {
 			*crc = symversion(__start___kcrctab_gpl,
 					  (ks - __start___ksymtab_gpl));
-			return ks->value;
+			return ks;
 		}
 	}
 	ks = lookup_symbol(name, __start___ksymtab_gpl_future,
@@ -209,7 +211,7 @@ static unsigned long __find_symbol(const
 		}
 		*crc = symversion(__start___kcrctab_gpl_future,
 				  (ks - __start___ksymtab_gpl_future));
-		return ks->value;
+		return ks;
 	}
 
 	ks = lookup_symbol(name, __start___ksymtab_unused,
@@ -218,7 +220,7 @@ static unsigned long __find_symbol(const
 		printk_unused_warning(name);
 		*crc = symversion(__start___kcrctab_unused,
 				  (ks - __start___ksymtab_unused));
-		return ks->value;
+		return ks;
 	}
 
 	if (gplok)
@@ -228,7 +230,7 @@ static unsigned long __find_symbol(const
 		printk_unused_warning(name);
 		*crc = symversion(__start___kcrctab_unused_gpl,
 				  (ks - __start___ksymtab_unused_gpl));
-		return ks->value;
+		return ks;
 	}
 
 	/* Now try modules. */
@@ -237,7 +239,7 @@ static unsigned long __find_symbol(const
 		ks = lookup_symbol(name, mod->syms, mod->syms + mod->num_syms);
 		if (ks) {
 			*crc = symversion(mod->crcs, (ks - mod->syms));
-			return ks->value;
+			return ks;
 		}
 
 		if (gplok) {
@@ -246,14 +248,14 @@ static unsigned long __find_symbol(const
 			if (ks) {
 				*crc = symversion(mod->gpl_crcs,
 						  (ks - mod->gpl_syms));
-				return ks->value;
+				return ks;
 			}
 		}
 		ks = lookup_symbol(name, mod->unused_syms, mod->unused_syms + mod->num_unused_syms);
 		if (ks) {
 			printk_unused_warning(name);
 			*crc = symversion(mod->unused_crcs, (ks - mod->unused_syms));
-			return ks->value;
+			return ks;
 		}
 
 		if (gplok) {
@@ -263,7 +265,7 @@ static unsigned long __find_symbol(const
 				printk_unused_warning(name);
 				*crc = symversion(mod->unused_gpl_crcs,
 						  (ks - mod->unused_gpl_syms));
-				return ks->value;
+				return ks;
 			}
 		}
 		ks = lookup_symbol(name, mod->gpl_future_syms,
@@ -281,11 +283,92 @@ static unsigned long __find_symbol(const
 			}
 			*crc = symversion(mod->gpl_future_crcs,
 					  (ks - mod->gpl_future_syms));
-			return ks->value;
+			return ks;
 		}
 	}
 	DEBUGP("Failed to find symbol %s\n", name);
-	return 0;
+ 	return NULL;
+}
+
+/* Find namespace in a single namespace table */
+static const struct module_ns *lookup_namespace(const struct module_ns *start,
+						const struct module_ns *stop,
+		 			  	const char *name,
+						const char *reqname, int *n)
+{
+	const struct module_ns *ns;
+	for (ns = start; ns < stop; ns++)
+		if (!strcmp(name, ns->name)) {
+			(*n)++;
+			if (!strcmp(reqname, ns->allowed))
+				return ns;
+		}
+	return NULL;
+}
+
+/* Search all namespace tables for a namespace */
+static const struct module_ns *find_namespace(const char *name,
+					      const char *reqname, int *n,
+					      const struct module **modp)
+{
+	const struct module_ns *ns;
+	struct module *mod;
+	*modp = NULL;
+	ns = lookup_namespace(__start___knamespace, __stop___knamespace,
+				name, reqname, n);
+	if (ns)
+		return ns;
+	list_for_each_entry(mod, &modules, list) {
+		ns = lookup_namespace(mod->knamespace,
+				      mod->knamespace + mod->num_knamespaces,
+				      name, reqname, n);
+		if (ns) {
+			*modp = mod;
+			return ns;
+		}
+	}
+	return NULL;
+}
+
+/* Look up a symbol and check namespace */
+static unsigned long find_symbol(const char *name,
+				   struct module **owner,
+				   const unsigned long **crc,
+				   int gplok,
+			           struct module *requester)
+{
+	const struct kernel_symbol *ks;
+	ks = do_find_symbol(name, owner, crc, gplok);
+	if (!ks)
+		return 0;
+	/* When the symbol has a name space check if the requesting module
+	   is white listed as allowed. */
+	if (ks->namespace) {
+		int n = 0;
+		const struct module_ns *ns;
+		const struct module *mod;
+		ns = find_namespace(ks->namespace, requester->name, &n, &mod);
+		if (!ns) {
+			if (n > 0)
+				printk("module %s not allowed to "
+				       "reference namespace `%s' for %s\n",
+					requester->name, ks->namespace, name);
+			else
+				printk("%s referencing undeclared "
+				       "namespace `%s' for %s\n",
+					requester->name, ks->namespace, name);
+			return 0;
+		}
+		/* Only allow name space declarations in the symbol's own
+		   module. */
+		if (mod != *owner) {
+			printk("namespace `%s' for symbol `%s' defined "
+			       "in wrong module `%s'\n",
+				name, name, mod->name);
+			return 0;
+		}
+	}
+	return ks->value;
 }
 
 /* Search for module by name: must hold module_mutex. */
@@ -750,18 +833,18 @@ static void print_unload_info(struct seq
 		seq_printf(m, "-");
 }
 
-void __symbol_put(const char *symbol)
+void do_symbol_put(const char *symbol, struct module *caller)
 {
 	struct module *owner;
 	const unsigned long *crc;
 
 	preempt_disable();
-	if (!__find_symbol(symbol, &owner, &crc, 1))
+	if (!find_symbol(symbol, &owner, &crc, 1, caller))
 		BUG();
 	module_put(owner);
 	preempt_enable();
 }
-EXPORT_SYMBOL(__symbol_put);
+EXPORT_SYMBOL(do_symbol_put);
 
 void symbol_put_addr(void *addr)
 {
@@ -902,7 +985,7 @@ static inline int check_modstruct_versio
 	const unsigned long *crc;
 	struct module *owner;
 
-	if (!__find_symbol("struct_module", &owner, &crc, 1))
+	if (!find_symbol("struct_module", &owner, &crc, 1, mod))
 		BUG();
 	return check_version(sechdrs, versindex, "struct_module", mod,
 			     crc);
@@ -949,8 +1032,8 @@ static unsigned long resolve_symbol(Elf_
 	unsigned long ret;
 	const unsigned long *crc;
 
-	ret = __find_symbol(name, &owner, &crc,
-			!(mod->taints & TAINT_PROPRIETARY_MODULE));
+	ret = find_symbol(name, &owner, &crc,
+			!(mod->taints & TAINT_PROPRIETARY_MODULE), mod);
 	if (ret) {
 		/* use_module can fail due to OOM, or module unloading */
 		if (!check_version(sechdrs, versindex, name, mod, crc) ||
@@ -1320,21 +1403,21 @@ static void free_module(struct module *m
 	module_free(mod, mod->module_core);
 }
 
-void *__symbol_get(const char *symbol)
+void *do_symbol_get(const char *symbol, struct module *caller)
 {
 	struct module *owner;
 	unsigned long value;
 	const unsigned long *crc;
 
 	preempt_disable();
-	value = __find_symbol(symbol, &owner, &crc, 1);
+	value = find_symbol(symbol, &owner, &crc, 1, caller);
 	if (value && !strong_try_module_get(owner))
 		value = 0;
 	preempt_enable();
 
 	return (void *)value;
 }
-EXPORT_SYMBOL_GPL(__symbol_get);
+EXPORT_SYMBOL_GPL(do_symbol_get);
 
 /*
  * Ensure that an exported symbol [global namespace] does not already exist
@@ -1348,14 +1431,14 @@ static int verify_export_symbols(struct 
 	const unsigned long *crc;
 
 	for (i = 0; i < mod->num_syms; i++)
-		if (__find_symbol(mod->syms[i].name, &owner, &crc, 1)) {
+	        if (find_symbol(mod->syms[i].name, &owner, &crc, 1, mod)) {
 			name = mod->syms[i].name;
 			ret = -ENOEXEC;
 			goto dup;
 		}
 
 	for (i = 0; i < mod->num_gpl_syms; i++)
-		if (__find_symbol(mod->gpl_syms[i].name, &owner, &crc, 1)) {
+	        if (find_symbol(mod->gpl_syms[i].name, &owner, &crc, 1, mod)) {
 			name = mod->gpl_syms[i].name;
 			ret = -ENOEXEC;
 			goto dup;
@@ -1675,6 +1758,7 @@ static struct module *load_module(void _
 	unsigned int unusedgplcrcindex;
 	unsigned int markersindex;
 	unsigned int markersstringsindex;
+	unsigned int namespaceindex;
 	struct module *mod;
 	long err = 0;
 	void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -1771,6 +1855,7 @@ static struct module *load_module(void _
 #ifdef ARCH_UNWIND_SECTION_NAME
 	unwindex = find_sec(hdr, sechdrs, secstrings, ARCH_UNWIND_SECTION_NAME);
 #endif
+	namespaceindex = find_sec(hdr, sechdrs, secstrings, "__knamespace");
 
 	/* Don't keep modinfo section */
 	sechdrs[infoindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
@@ -1930,6 +2015,12 @@ static struct module *load_module(void _
 	if (unusedgplcrcindex)
 		mod->unused_crcs = (void *)sechdrs[unusedgplcrcindex].sh_addr;
 
+	if (namespaceindex) {
+		mod->knamespace = (void *)sechdrs[namespaceindex].sh_addr;
+		mod->num_knamespaces = sechdrs[namespaceindex].sh_size /
+					sizeof(struct module_ns);
+	}
+
 #ifdef CONFIG_MODVERSIONS
 	if ((mod->num_syms && !crcindex) ||
 	    (mod->num_gpl_syms && !gplcrcindex) ||
Index: linux/include/asm-generic/vmlinux.lds.h
===================================================================
--- linux.orig/include/asm-generic/vmlinux.lds.h
+++ linux/include/asm-generic/vmlinux.lds.h
@@ -127,6 +127,13 @@
 		VMLINUX_SYMBOL(__stop___kcrctab_gpl_future) = .;	\
 	}								\
 									\
+	/* Kernel symbol table: namespaces */				\
+	__knamespace : AT(ADDR(__knamespace) - LOAD_OFFSET) { 		\
+		VMLINUX_SYMBOL(__start___knamespace) = .;		\
+		*(__knamespace)						\
+		VMLINUX_SYMBOL(__stop___knamespace) = .;		\
+	}								\
+									\
 	/* Kernel symbol table: strings */				\
         __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {	\
 		*(__ksymtab_strings)					\
Index: linux/arch/arm/kernel/armksyms.c
===================================================================
--- linux.orig/arch/arm/kernel/armksyms.c
+++ linux/arch/arm/kernel/armksyms.c
@@ -52,7 +52,7 @@ extern void fp_enter(void);
  * This has a special calling convention; it doesn't
  * modify any of the usual registers, except for LR.
  */
-#define EXPORT_CRC_ALIAS(sym) __CRC_SYMBOL(sym, "")
+#define EXPORT_CRC_ALIAS(sym) __CRC_SYMBOL(sym, "", "", "")
 
 #define EXPORT_SYMBOL_ALIAS(sym,orig)		\
  EXPORT_CRC_ALIAS(sym)				\

[PATCH RFC] [2/9] Fix duplicate symbol check to also check future gpl and unused symbols

[Andi Kleen]

This seems to have been forgotten earlier. Right now it was possible
for a normal symbol to override a future gpl symbol and similar.
I restructured the code a bit to avoid too much duplicated code.

---
 kernel/module.c |   45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

Index: linux/kernel/module.c
===================================================================
--- linux.orig/kernel/module.c
+++ linux/kernel/module.c
@@ -1430,33 +1430,36 @@ EXPORT_SYMBOL_GPL(do_symbol_get);
  * Ensure that an exported symbol [global namespace] does not already exist
  * in the kernel or in some other module's exported symbol table.
  */
-static int verify_export_symbols(struct module *mod)
+
+static int check_duplicate(const struct kernel_symbol *syms, int num, struct module *owner)
 {
-	const char *name = NULL;
-	unsigned long i, ret = 0;
-	struct module *owner;
+	int i;
 	const unsigned long *crc;
 
-	for (i = 0; i < mod->num_syms; i++)
-	        if (find_symbol(mod->syms[i].name, &owner, &crc, 1, mod)) {
-			name = mod->syms[i].name;
-			ret = -ENOEXEC;
-			goto dup;
-		}
-
-	for (i = 0; i < mod->num_gpl_syms; i++)
-	        if (find_symbol(mod->gpl_syms[i].name, &owner, &crc, 1, mod)) {
-			name = mod->gpl_syms[i].name;
-			ret = -ENOEXEC;
-			goto dup;
+	for (i = 0; i < num; i++)
+	        if (find_symbol(syms[i].name, &owner, &crc, 1, owner)) {
+			printk(KERN_ERR "%s: exports duplicate symbol %s (owned by %s)\n",
+				owner->name, syms[i].name, module_name(owner));
+			return -ENOEXEC;
 		}
+	return 0;
+}
 
-dup:
+static int verify_export_symbols(struct module *mod)
+{
+	int ret = check_duplicate(mod->syms, mod->num_syms, mod);
 	if (ret)
-		printk(KERN_ERR "%s: exports duplicate symbol %s (owned by %s)\n",
-			mod->name, name, module_name(owner));
-
-	return ret;
+		return ret;
+	ret = check_duplicate(mod->gpl_syms, mod->num_gpl_syms, mod);
+	if (ret)
+		return ret;
+	ret = check_duplicate(mod->unused_syms, mod->num_unused_syms, mod);
+	if (ret)
+		return ret;
+	ret = check_duplicate(mod->unused_gpl_syms, mod->num_unused_gpl_syms, mod);
+	if (ret)
+		return ret;
+	return check_duplicate(mod->gpl_future_syms, mod->num_gpl_future_syms, mod);
 }
 
 /* Change all symbols so that sh_value encodes the pointer directly. */

[PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like

[Andi Kleen]

This way gcc can warn for wrong format strings

---
 scripts/mod/modpost.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: linux/scripts/mod/modpost.c
===================================================================
--- linux.orig/scripts/mod/modpost.c
+++ linux/scripts/mod/modpost.c
@@ -33,7 +33,9 @@ enum export {
 	export_unused_gpl, export_gpl_future, export_unknown
 };
 
-void fatal(const char *fmt, ...)
+#define PRINTF __attribute__ ((format (printf, 1, 2)))
+
+PRINTF void fatal(const char *fmt, ...)
 {
 	va_list arglist;
 
@@ -46,7 +48,7 @@ void fatal(const char *fmt, ...)
 	exit(1);
 }
 
-void warn(const char *fmt, ...)
+PRINTF void warn(const char *fmt, ...)
 {
 	va_list arglist;
 
@@ -57,7 +59,7 @@ void warn(const char *fmt, ...)
 	va_end(arglist);
 }
 
-void merror(const char *fmt, ...)
+PRINTF void merror(const char *fmt, ...)
 {
 	va_list arglist;
 

[PATCH RFC] [4/9] modpost: Fix format string warnings

[Andi Kleen]

Fix wrong format strings in modpost exposed by the previous patch.
Including one missing argument -- some random data was printed instead.

---
 scripts/mod/modpost.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Index: linux/scripts/mod/modpost.c
===================================================================
--- linux.orig/scripts/mod/modpost.c
+++ linux/scripts/mod/modpost.c
@@ -388,7 +388,7 @@ static int parse_elf(struct elf_info *in
 
 	/* Check if file offset is correct */
 	if (hdr->e_shoff > info->size) {
-		fatal("section header offset=%u in file '%s' is bigger then filesize=%lu\n", hdr->e_shoff, filename, info->size);
+		fatal("section header offset=%lu in file '%s' is bigger then filesize=%lu\n", (unsigned long)hdr->e_shoff, filename, info->size);
 		return 0;
 	}
 
@@ -409,7 +409,7 @@ static int parse_elf(struct elf_info *in
 		const char *secname;
 
 		if (sechdrs[i].sh_offset > info->size) {
-			fatal("%s is truncated. sechdrs[i].sh_offset=%u > sizeof(*hrd)=%ul\n", filename, (unsigned int)sechdrs[i].sh_offset, sizeof(*hdr));
+			fatal("%s is truncated. sechdrs[i].sh_offset=%lu > sizeof(*hrd)=%lu\n", filename, (unsigned long)sechdrs[i].sh_offset, sizeof(*hdr));
 			return 0;
 		}
 		secname = secstrings + sechdrs[i].sh_name;
@@ -907,7 +907,8 @@ static void warn_sec_mismatch(const char
 		     "before '%s' (at offset -0x%llx)\n",
 		     modname, fromsec, (unsigned long long)r.r_offset,
 		     secname, refsymname,
-		     elf->strtab + after->st_name);
+		     elf->strtab + after->st_name,
+		     (unsigned long long)r.r_offset);
 	} else {
 		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s\n",
 		     modname, fromsec, (unsigned long long)r.r_offset,

[PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost

[Andi Kleen]

When passing an file name > 1k the stack could be overflowed.
Not really a security issue, but still better plugged.


---
 scripts/mod/modpost.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux/scripts/mod/modpost.c
===================================================================
--- linux.orig/scripts/mod/modpost.c
+++ linux/scripts/mod/modpost.c
@@ -1656,7 +1656,6 @@ int main(int argc, char **argv)
 {
 	struct module *mod;
 	struct buffer buf = { };
-	char fname[SZ];
 	char *kernel_read = NULL, *module_read = NULL;
 	char *dump_write = NULL;
 	int opt;
@@ -1709,6 +1708,8 @@ int main(int argc, char **argv)
 	err = 0;
 
 	for (mod = modules; mod; mod = mod->next) {
+		char fname[strlen(mod->name) + 10];
+
 		if (mod->skip)
 			continue;
 

[PATCH RFC] [6/9] Implement namespace checking in modpost

[Andi Kleen]

This checks the namespaces at build time in modpost

---
 scripts/mod/modpost.c |  344 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 317 insertions(+), 27 deletions(-)

Index: linux/scripts/mod/modpost.c
===================================================================
--- linux.orig/scripts/mod/modpost.c
+++ linux/scripts/mod/modpost.c
@@ -1,8 +1,9 @@
-/* Postprocess module symbol versions
+/* Postprocess module symbol versions and do various other module checks.
  *
  * Copyright 2003       Kai Germaschewski
  * Copyright 2002-2004  Rusty Russell, IBM Corporation
  * Copyright 2006       Sam Ravnborg
+ * Copyright 2007	Andi Kleen, SUSE Labs (changes licensed GPLv2 only)
  * Based in part on module-init-tools/depmod.c,file2alias
  *
  * This software may be used and distributed according to the terms
@@ -12,9 +13,13 @@
  */
 
 #include <ctype.h>
+#include <assert.h>
 #include "modpost.h"
 #include "../../include/linux/license.h"
 
+#define NS_SEPARATOR '.'
+#define NS_SEPARATOR_STRING "."
+
 /* Are we using CONFIG_MODVERSIONS? */
 int modversions = 0;
 /* Warn about undefined symbols? (do so if we have vmlinux) */
@@ -27,6 +32,9 @@ static int external_module = 0;
 static int vmlinux_section_warnings = 1;
 /* Only warn about unresolved symbols */
 static int warn_unresolved = 0;
+/* Fixing those would cause too many ifdefs -- off by default. */
+static int warn_missing_modules = 0;
+
 /* How a symbol is exported */
 enum export {
 	export_plain,      export_unused,     export_gpl,
@@ -105,19 +113,43 @@ static struct module *find_module(char *
 	return mod;
 }
 
-static struct module *new_module(char *modname)
+static const char *basename(const char *s)
+{
+	char *p = strrchr(s, '/');
+	if (p)
+		return p + 1;
+	return s;
+}
+
+static struct module *find_module_base(char *modname)
 {
 	struct module *mod;
-	char *p, *s;
 
-	mod = NOFAIL(malloc(sizeof(*mod)));
-	memset(mod, 0, sizeof(*mod));
-	p = NOFAIL(strdup(modname));
+	for (mod = modules; mod; mod = mod->next) {
+		if (strcmp(basename(mod->name), modname) == 0)
+			break;
+	}
+	return mod;
+}
 
+static void strip_o(char *p)
+{
+	char *s;
 	/* strip trailing .o */
 	if ((s = strrchr(p, '.')) != NULL)
 		if (strcmp(s, ".o") == 0)
 			*s = '\0';
+}
+
+static struct module *new_module(char *modname)
+{
+	struct module *mod;
+	char *p;
+
+	mod = NOFAIL(malloc(sizeof(*mod)));
+	memset(mod, 0, sizeof(*mod));
+	p = NOFAIL(strdup(modname));
+	strip_o(p);
 
 	/* add to list */
 	mod->name = p;
@@ -132,10 +164,12 @@ static struct module *new_module(char *m
  * struct symbol is also used for lists of unresolved symbols */
 
 #define SYMBOL_HASH_SIZE 1024
+#define NSALLOW_HASH_SIZE 64
 
 struct symbol {
 	struct symbol *next;
 	struct module *module;
+	const char *namespace;
 	unsigned int crc;
 	int crc_valid;
 	unsigned int weak:1;
@@ -147,10 +181,19 @@ struct symbol {
 	char name[0];
 };
 
+struct nsallow {
+	struct nsallow *next;
+	struct module *mod;
+	struct module *orig;
+	int ref;
+	char name[0];
+};
+
 static struct symbol *symbolhash[SYMBOL_HASH_SIZE];
+static struct nsallow *nsallowhash[NSALLOW_HASH_SIZE];
 
 /* This is based on the hash agorithm from gdbm, via tdb */
-static inline unsigned int tdb_hash(const char *name)
+static unsigned int tdb_hash(const char *name)
 {
 	unsigned value;	/* Used to compute the hash value.  */
 	unsigned   i;	/* Used to cycle through random values. */
@@ -192,21 +235,67 @@ static struct symbol *new_symbol(const c
 	return new;
 }
 
-static struct symbol *find_symbol(const char *name)
+static struct symbol *find_symbol(const char *name, const char *ns)
 {
-	struct symbol *s;
+	struct symbol *s, *match;
 
 	/* For our purposes, .foo matches foo.  PPC64 needs this. */
 	if (name[0] == '.')
 		name++;
 
+	match = NULL;
 	for (s = symbolhash[tdb_hash(name) % SYMBOL_HASH_SIZE]; s; s=s->next) {
+		if (strcmp(s->name, name) == 0) {
+			match = s;
+			if (ns && s->namespace && strcmp(s->namespace, ns))
+				continue;
+			return s;
+		}
+	}
+	return ns ? NULL : match;
+}
+
+static struct nsallow *find_nsallow(const char *name, struct module *mod)
+{
+	struct nsallow *s;
+
+	for (s = nsallowhash[tdb_hash(name)%NSALLOW_HASH_SIZE]; s; s=s->next) {
+		if (strcmp(s->name, name) == 0 && s->mod == mod)
+			return s;
+	}
+	return NULL;
+}
+
+static struct nsallow *find_nsallow_name(const char *name)
+{
+	struct nsallow *s;
+
+	for (s = nsallowhash[tdb_hash(name)%NSALLOW_HASH_SIZE]; s; s=s->next) {
 		if (strcmp(s->name, name) == 0)
 			return s;
 	}
 	return NULL;
 }
 
+static struct nsallow *
+new_nsallow(const char *name, struct module *module, struct module *orig)
+{
+	unsigned int hash;
+	struct nsallow *new;
+	unsigned len;
+
+	len = sizeof(struct nsallow) + strlen(name) + 1;
+	new = NOFAIL(malloc(len));
+	new->mod = module;
+	new->orig = orig;
+	new->ref = 0;
+	strcpy(new->name, name);
+	hash = tdb_hash(name) % NSALLOW_HASH_SIZE;
+	new->next = nsallowhash[hash];
+	nsallowhash[hash] = new;
+	return new;
+}
+
 static struct {
 	const char *str;
 	enum export export;
@@ -253,27 +342,103 @@ static enum export export_from_sec(struc
 		return export_unknown;
 }
 
+/* Check if all the name space that allows referencing the symbol's
+   namespace is in the same module as the export. This makes sure a
+   module doesn't allow itself into a namespace (the kernel checks
+   this too) */
+static void check_export_namespace(struct symbol *s)
+{
+	const char *namespace = s->namespace;
+	struct nsallow *nsa = find_nsallow_name(namespace);
+	if (!nsa) {
+		warn("%s: namespace %s used for export `%s', but no module "
+		     "for it allowed\n", s->module->name, namespace, s->name);
+	}
+	for (; nsa; nsa = nsa->next) {
+		if (strcmp(nsa->name, namespace))
+			continue;
+		if (nsa->orig == s->module) {
+			nsa->ref++;
+			return;
+		}
+	}
+	merror("No module allowed for namespace %s in %s exporting %s\n",
+		namespace,
+		s->module->name,
+		s->name);
+}
+
+static void check_nsallow_referenced(void)
+{
+	int i;
+	struct nsallow *nsa;
+	for (i = 0; i < NSALLOW_HASH_SIZE; i++) {
+		for (nsa = nsallowhash[i]; nsa; nsa = nsa->next)
+			if (nsa->ref == 0 && !nsa->mod->skip) {
+				warn("%s: namespace %s allowed for module %s, "
+				     "but no exports using it\n",
+				     nsa->orig->name,
+				     nsa->name,
+				     nsa->mod->name);
+			}
+	}
+}
+
+static const char *sep_ns(const char **name)
+{
+	char *p;
+	const char *s = strchr(*name, NS_SEPARATOR);
+	if (!s)
+		return NULL;
+	*name = NOFAIL(strdup(*name));
+	p = strchr(*name, NS_SEPARATOR);
+	*p = '\0';
+	return p + 1;
+}
+
+static int deep_streq(const char *a, const char *b)
+{
+	if (a == b)
+		return 1;
+	if (!a || !b)
+		return 0;
+	return !strcmp(a, b);
+}
+
 /**
  * Add an exported symbol - it may have already been added without a
  * CRC, in this case just update the CRC
  **/
-static struct symbol *sym_add_exported(const char *name, struct module *mod,
+static struct symbol *sym_add_exported(const char *oname, struct module *mod,
 				       enum export export)
 {
-	struct symbol *s = find_symbol(name);
+	const char *name = oname;
+	const char *namespace = sep_ns(&name);
+	struct symbol *s = find_symbol(name, namespace);
 
 	if (!s) {
 		s = new_symbol(name, mod, export);
+		s->namespace = namespace;
 	} else {
 		if (!s->preloaded) {
 			warn("%s: '%s' exported twice. Previous export "
-			     "was in %s%s\n", mod->name, name,
+			     "was in %s%s\n", mod->name, oname,
 			     s->module->name,
 			     is_vmlinux(s->module->name) ?"":".ko");
+			if (!deep_streq(s->namespace, namespace)) {
+				warn("%s: New namespace '%s' of '%s' doesn't "
+				     "match existing namespace '%s'\n",
+				     s->module->name,
+				     namespace ?: "",
+				     s->name ?: "???",
+				     s->namespace ?: "");
+			}
 		} else {
 			/* In case Modules.symvers was out of date */
 			s->module = mod;
 		}
+		if (name != oname)
+			free((char *)name);
 	}
 	s->preloaded = 0;
 	s->vmlinux   = is_vmlinux(mod->name);
@@ -282,13 +447,58 @@ static struct symbol *sym_add_exported(c
 	return s;
 }
 
-static void sym_update_crc(const char *name, struct module *mod,
+static struct module *
+nsmodule(const char *name, int sep, const char *namespace, struct module *orig)
+{
+	struct module *mod;
+	char *modname = NOFAIL(malloc(sep + 1));
+
+	memcpy(modname, name, sep);
+	modname[sep] = 0;
+	mod = find_module_base(modname);
+	if (!mod) {
+		if (warn_missing_modules)
+			warn("%s: Namespace allow for %s references unknown"
+		    	 " module %.*s\n", orig->name, namespace, sep, name);
+		mod = new_module(modname);
+		mod->skip = 1;
+	} else {
+		free(modname);
+	}
+	return mod;
+}
+
+static void sym_add_nsallow(const char *name, struct module *orig)
+{
+	struct module *mod;
+	int sep;
+	const char *namespace;
+
+	sep = strcspn(name, NS_SEPARATOR_STRING);
+	if (name[sep] == 0 || sep == 0) {
+		warn("%s: Namespace allow '%s' incorrectly formatted\n",
+			orig->name, name);
+		return;
+	}
+	namespace = name + sep + 1;
+	mod = nsmodule(name, sep, namespace, orig);
+	if (!find_nsallow(namespace, mod))
+		new_nsallow(namespace, mod, orig);
+}
+
+static void sym_update_crc(const char *oname, struct module *mod,
 			   unsigned int crc, enum export export)
 {
-	struct symbol *s = find_symbol(name);
+	const char *name = oname;
+	const char *namespace = sep_ns(&name);
+	struct symbol *s = find_symbol(name, namespace);
 
-	if (!s)
+	if (!s) {
 		s = new_symbol(name, mod, export);
+	} else if (oname != name) {
+		assert(deep_streq(s->namespace, namespace));
+		free((char *)name);
+	}
 	s->crc = crc;
 	s->crc_valid = 1;
 }
@@ -456,6 +666,22 @@ static void parse_elf_finish(struct elf_
 
 #define CRC_PFX     MODULE_SYMBOL_PREFIX "__crc_"
 #define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_"
+#define NSALLOW_PFX MODULE_SYMBOL_PREFIX "__knamespace_"
+
+
+static void handle_nsallow(struct module *mod, struct elf_info *info,
+			       Elf_Sym *sym, const char *symname)
+{
+	switch (sym->st_shndx) {
+	case SHN_COMMON:
+	case SHN_ABS:
+	case SHN_UNDEF:
+		break;
+	default:
+		if (!strncmp(symname, NSALLOW_PFX, sizeof(NSALLOW_PFX)-1))
+			sym_add_nsallow(symname + sizeof(NSALLOW_PFX) - 1, mod);
+	}
+}
 
 static void handle_modversions(struct module *mod, struct elf_info *info,
 			       Elf_Sym *sym, const char *symname)
@@ -507,17 +733,25 @@ static void handle_modversions(struct mo
 #endif
 
 		if (memcmp(symname, MODULE_SYMBOL_PREFIX,
-			   strlen(MODULE_SYMBOL_PREFIX)) == 0)
-			mod->unres = alloc_symbol(symname +
-						  strlen(MODULE_SYMBOL_PREFIX),
+			   strlen(MODULE_SYMBOL_PREFIX)) == 0) {
+			const int plen = strlen(MODULE_SYMBOL_PREFIX);
+			const char *name = symname + plen;
+			const char *namespace = sep_ns(&name);
+
+			mod->unres = alloc_symbol(name,
 						  ELF_ST_BIND(sym->st_info) == STB_WEAK,
 						  mod->unres);
+			mod->unres->namespace = namespace;
+		}
 		break;
 	default:
 		/* All exported symbols */
 		if (memcmp(symname, KSYMTAB_PFX, strlen(KSYMTAB_PFX)) == 0) {
-			sym_add_exported(symname + strlen(KSYMTAB_PFX), mod,
-					export);
+			struct symbol *s;
+			s = sym_add_exported(symname + strlen(KSYMTAB_PFX), mod,
+					     export);
+			if (s->namespace)
+				check_export_namespace(s);
 		}
 		if (strcmp(symname, MODULE_SYMBOL_PREFIX "init_module") == 0)
 			mod->has_init = 1;
@@ -1261,11 +1495,16 @@ static void read_symbols(char *modname)
 	struct elf_info info = { };
 	Elf_Sym *sym;
 
+	char m[strlen(modname) + 1];
+	strcpy(m, modname);
+	strip_o(m);
+
+	mod = find_module(m);
+	assert(mod != NULL);
+
 	if (!parse_elf(&info, modname))
 		return;
 
-	mod = new_module(modname);
-
 	/* When there's no vmlinux, don't print warnings about
 	 * unresolved symbols (since there'll be too many ;) */
 	if (is_vmlinux(modname)) {
@@ -1285,6 +1524,12 @@ static void read_symbols(char *modname)
 					   "license", license);
 	}
 
+	/* First process all nsallows to check them against the symbols */
+	for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
+		symname = info.strtab + sym->st_name;
+		handle_nsallow(mod, &info, sym, symname);
+	}
+
 	for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
 		symname = info.strtab + sym->st_name;
 
@@ -1384,13 +1629,39 @@ static void check_for_unused(enum export
 	}
 }
 
+/* Check if symbol reference to S from module SYMMOD is allowed in namespace */
+static void check_symbol_ns(struct symbol *s, struct module *symmod,
+			    const char *symmodname)
+{
+	const char *namespace = s->namespace;
+	struct nsallow *nsa;
+	int sep;
+
+	if (is_vmlinux(symmodname))
+		return;
+	nsa = find_nsallow(namespace, symmod);
+	if (nsa)
+		return;
+	sep = strcspn(s->name, NS_SEPARATOR_STRING);
+	if (!find_nsallow_name(namespace)) {
+		warn("%s: Unknown namespace %s referenced from `%.*s'\n",
+		     symmodname, namespace, sep, s->name);
+	} else {
+		merror("%s: module not allowed to reference symbol "
+		       "'%.*s' in namespace %s\n",
+		       symmodname,
+		       sep, s->name,
+		       namespace);
+	}
+}
+
 static void check_exports(struct module *mod)
 {
 	struct symbol *s, *exp;
 
 	for (s = mod->unres; s; s = s->next) {
 		const char *basename;
-		exp = find_symbol(s->name);
+		exp = find_symbol(s->name, s->namespace);
 		if (!exp || exp->module == mod)
 			continue;
 		basename = strrchr(mod->name, '/');
@@ -1398,6 +1669,8 @@ static void check_exports(struct module 
 			basename++;
 		else
 			basename = mod->name;
+		if (s->namespace)
+			check_symbol_ns(s, mod, basename);
 		if (!mod->gpl_compatible)
 			check_for_gpl_usage(exp->export, basename, exp->name);
 		check_for_unused(exp->export, basename, exp->name);
@@ -1437,14 +1710,18 @@ static int add_versions(struct buffer *b
 	int err = 0;
 
 	for (s = mod->unres; s; s = s->next) {
-		exp = find_symbol(s->name);
+		exp = find_symbol(s->name, s->namespace);
 		if (!exp || exp->module == mod) {
 			if (have_vmlinux && !s->weak) {
 				if (warn_unresolved) {
-					warn("\"%s\" [%s.ko] undefined!\n",
+					warn("\"%s%s%s\" [%s.ko] undefined!\n",
+					 	s->namespace ?: "",
+						s->namespace ? ":" : "",
 					     s->name, mod->name);
 				} else {
-					merror("\"%s\" [%s.ko] undefined!\n",
+					merror("\"%s%s%s\" [%s.ko] undefined!\n",
+					 	s->namespace ?: "",
+						s->namespace ? ":" : "",
 					          s->name, mod->name);
 					err = 1;
 				}
@@ -1606,7 +1883,7 @@ static void read_dump(const char *fname,
 			if (is_vmlinux(modname)) {
 				have_vmlinux = 1;
 			}
-			mod = new_module(NOFAIL(strdup(modname)));
+			mod = new_module(modname);
 			mod->skip = 1;
 		}
 		s = sym_add_exported(symname, mod, export_no(export));
@@ -1660,6 +1937,7 @@ int main(int argc, char **argv)
 	char *dump_write = NULL;
 	int opt;
 	int err;
+	int c;
 
 	while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
 		switch(opt) {
@@ -1685,6 +1963,9 @@ int main(int argc, char **argv)
 			case 'w':
 				warn_unresolved = 1;
 				break;
+			case 'M':
+				warn_missing_modules = 1;
+				break;
 			default:
 				exit(1);
 		}
@@ -1695,6 +1976,12 @@ int main(int argc, char **argv)
 	if (module_read)
 		read_dump(module_read, 0);
 
+	for (c = optind; c < argc; c++) {
+		char s[strlen(argv[c]) + 1];
+		strcpy(s, argv[c]);
+		new_module(s);
+	}
+
 	while (optind < argc) {
 		read_symbols(argv[optind++]);
 	}
@@ -1705,6 +1992,9 @@ int main(int argc, char **argv)
 		check_exports(mod);
 	}
 
+	if (warn_unresolved)
+		check_nsallow_referenced();
+
 	err = 0;
 
 	for (mod = modules; mod; mod = mod->next) {

[PATCH RFC] [7/9] Convert TCP exports into namespaces

[Andi Kleen]

I defined two namespaces: tcp for TCP internals which are only used by 
tcp_ipv6.ko And tcpcong for exports used by the TCP congestion modules

No need to export any TCP internals to anybody else. So express this in a namespace.

I admit I'm not 100% sure tcpcong makes sense -- there might be a legitimate
need to have external out of tree congestion modules. They seem nearly like
drivers, but only nearly. If that was deemed the case it would be possible to 
remove tcpcong again to allow everybody to access this.

This implicitely turns all exports into GPL only, but that won't matter
because all modules allowed to import TCP functions are GPLed.

---
 net/ipv4/tcp.c           |   71 +++++++++++++++++++++++++++--------------------
 net/ipv4/tcp_cong.c      |   14 ++++-----
 net/ipv4/tcp_input.c     |   12 +++----
 net/ipv4/tcp_ipv4.c      |   38 ++++++++++++-------------
 net/ipv4/tcp_minisocks.c |   12 +++----
 net/ipv4/tcp_output.c    |   12 +++----
 net/ipv4/tcp_timer.c     |    2 -
 7 files changed, 87 insertions(+), 74 deletions(-)

Index: linux/net/ipv4/tcp.c
===================================================================
--- linux.orig/net/ipv4/tcp.c
+++ linux/net/ipv4/tcp.c
@@ -275,21 +275,21 @@ DEFINE_SNMP_STAT(struct tcp_mib, tcp_sta
 
 atomic_t tcp_orphan_count = ATOMIC_INIT(0);
 
-EXPORT_SYMBOL_GPL(tcp_orphan_count);
+EXPORT_SYMBOL_NS(tcp, tcp_orphan_count);
 
 int sysctl_tcp_mem[3] __read_mostly;
 int sysctl_tcp_wmem[3] __read_mostly;
 int sysctl_tcp_rmem[3] __read_mostly;
 
-EXPORT_SYMBOL(sysctl_tcp_mem);
-EXPORT_SYMBOL(sysctl_tcp_rmem);
-EXPORT_SYMBOL(sysctl_tcp_wmem);
+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_mem);
+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_rmem);
+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_wmem);
 
 atomic_t tcp_memory_allocated;	/* Current allocated memory. */
 atomic_t tcp_sockets_allocated;	/* Current number of TCP sockets. */
 
-EXPORT_SYMBOL(tcp_memory_allocated);
-EXPORT_SYMBOL(tcp_sockets_allocated);
+EXPORT_SYMBOL_NS(tcp, tcp_memory_allocated);
+EXPORT_SYMBOL_NS(tcp, tcp_sockets_allocated);
 
 /*
  * Pressure flag: try to collapse.
@@ -299,7 +299,7 @@ EXPORT_SYMBOL(tcp_sockets_allocated);
  */
 int tcp_memory_pressure __read_mostly;
 
-EXPORT_SYMBOL(tcp_memory_pressure);
+EXPORT_SYMBOL_NS(tcp, tcp_memory_pressure);
 
 void tcp_enter_memory_pressure(void)
 {
@@ -309,7 +309,7 @@ void tcp_enter_memory_pressure(void)
 	}
 }
 
-EXPORT_SYMBOL(tcp_enter_memory_pressure);
+EXPORT_SYMBOL_NS(tcp, tcp_enter_memory_pressure);
 
 /*
  *	Wait for a TCP event.
@@ -1995,7 +1995,7 @@ int compat_tcp_setsockopt(struct sock *s
 	return do_tcp_setsockopt(sk, level, optname, optval, optlen);
 }
 
-EXPORT_SYMBOL(compat_tcp_setsockopt);
+EXPORT_SYMBOL_NS(tcp, compat_tcp_setsockopt);
 #endif
 
 /* Return information about state of tcp endpoint in API format. */
@@ -2061,7 +2061,7 @@ void tcp_get_info(struct sock *sk, struc
 	info->tcpi_total_retrans = tp->total_retrans;
 }
 
-EXPORT_SYMBOL_GPL(tcp_get_info);
+EXPORT_SYMBOL_NS(tcp, tcp_get_info);
 
 static int do_tcp_getsockopt(struct sock *sk, int level,
 		int optname, char __user *optval, int __user *optlen)
@@ -2174,7 +2174,7 @@ int compat_tcp_getsockopt(struct sock *s
 	return do_tcp_getsockopt(sk, level, optname, optval, optlen);
 }
 
-EXPORT_SYMBOL(compat_tcp_getsockopt);
+EXPORT_SYMBOL_NS(tcp, compat_tcp_getsockopt);
 #endif
 
 struct sk_buff *tcp_tso_segment(struct sk_buff *skb, int features)
@@ -2262,7 +2262,7 @@ struct sk_buff *tcp_tso_segment(struct s
 out:
 	return segs;
 }
-EXPORT_SYMBOL(tcp_tso_segment);
+EXPORT_SYMBOL_NS(tcp, tcp_tso_segment);
 
 #ifdef CONFIG_TCP_MD5SIG
 static unsigned long tcp_md5sig_users;
@@ -2298,7 +2298,7 @@ void tcp_free_md5sig_pool(void)
 		__tcp_free_md5sig_pool(pool);
 }
 
-EXPORT_SYMBOL(tcp_free_md5sig_pool);
+EXPORT_SYMBOL_NS(tcp, tcp_free_md5sig_pool);
 
 static struct tcp_md5sig_pool **__tcp_alloc_md5sig_pool(void)
 {
@@ -2371,7 +2371,7 @@ retry:
 	return pool;
 }
 
-EXPORT_SYMBOL(tcp_alloc_md5sig_pool);
+EXPORT_SYMBOL_NS(tcp, tcp_alloc_md5sig_pool);
 
 struct tcp_md5sig_pool *__tcp_get_md5sig_pool(int cpu)
 {
@@ -2384,14 +2384,14 @@ struct tcp_md5sig_pool *__tcp_get_md5sig
 	return (p ? *per_cpu_ptr(p, cpu) : NULL);
 }
 
-EXPORT_SYMBOL(__tcp_get_md5sig_pool);
+EXPORT_SYMBOL_NS(tcp, __tcp_get_md5sig_pool);
 
 void __tcp_put_md5sig_pool(void)
 {
 	tcp_free_md5sig_pool();
 }
 
-EXPORT_SYMBOL(__tcp_put_md5sig_pool);
+EXPORT_SYMBOL_NS(tcp, __tcp_put_md5sig_pool);
 #endif
 
 void tcp_done(struct sock *sk)
@@ -2409,7 +2409,7 @@ void tcp_done(struct sock *sk)
 	else
 		inet_csk_destroy_sock(sk);
 }
-EXPORT_SYMBOL_GPL(tcp_done);
+EXPORT_SYMBOL_NS(tcp, tcp_done);
 
 extern void __skb_cb_too_small_for_tcp(int, int);
 extern struct tcp_congestion_ops tcp_reno;
@@ -2524,15 +2524,28 @@ void __init tcp_init(void)
 	tcp_register_congestion_control(&tcp_reno);
 }
 
-EXPORT_SYMBOL(tcp_close);
-EXPORT_SYMBOL(tcp_disconnect);
-EXPORT_SYMBOL(tcp_getsockopt);
-EXPORT_SYMBOL(tcp_ioctl);
-EXPORT_SYMBOL(tcp_poll);
-EXPORT_SYMBOL(tcp_read_sock);
-EXPORT_SYMBOL(tcp_recvmsg);
-EXPORT_SYMBOL(tcp_sendmsg);
-EXPORT_SYMBOL(tcp_sendpage);
-EXPORT_SYMBOL(tcp_setsockopt);
-EXPORT_SYMBOL(tcp_shutdown);
-EXPORT_SYMBOL(tcp_statistics);
+EXPORT_SYMBOL_NS(tcp, tcp_close);
+EXPORT_SYMBOL_NS(tcp, tcp_disconnect);
+EXPORT_SYMBOL_NS(tcp, tcp_getsockopt);
+EXPORT_SYMBOL_NS(tcp, tcp_ioctl);
+EXPORT_SYMBOL_NS(tcp, tcp_poll);
+EXPORT_SYMBOL_NS(tcp, tcp_read_sock);
+EXPORT_SYMBOL_NS(tcp, tcp_recvmsg);
+EXPORT_SYMBOL_NS(tcp, tcp_sendmsg);
+EXPORT_SYMBOL_NS(tcp, tcp_sendpage);
+EXPORT_SYMBOL_NS(tcp, tcp_setsockopt);
+EXPORT_SYMBOL_NS(tcp, tcp_shutdown);
+EXPORT_SYMBOL_NS(tcp, tcp_statistics);
+
+MODULE_NAMESPACE_ALLOW(tcp, ipv6);
+MODULE_NAMESPACE_ALLOW(tcpcong, ipv6);
+
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_bic);
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_cubic);
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_westwood);
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_hyspeed);
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_hybla);
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_htcp);
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_vegas);
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_scalable);
+MODULE_NAMESPACE_ALLOW(tcpcong, tcp_lp);
Index: linux/net/ipv4/tcp_ipv4.c
===================================================================
--- linux.orig/net/ipv4/tcp_ipv4.c
+++ linux/net/ipv4/tcp_ipv4.c
@@ -163,7 +163,7 @@ int tcp_twsk_unique(struct sock *sk, str
 	return 0;
 }
 
-EXPORT_SYMBOL_GPL(tcp_twsk_unique);
+EXPORT_SYMBOL_NS(tcp, tcp_twsk_unique);
 
 /* This will initiate an outgoing connection. */
 int tcp_v4_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
@@ -845,7 +845,7 @@ struct tcp_md5sig_key *tcp_v4_md5_lookup
 	return tcp_v4_md5_do_lookup(sk, inet_sk(addr_sk)->daddr);
 }
 
-EXPORT_SYMBOL(tcp_v4_md5_lookup);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_md5_lookup);
 
 static struct tcp_md5sig_key *tcp_v4_reqsk_md5_lookup(struct sock *sk,
 						      struct request_sock *req)
@@ -913,7 +913,7 @@ int tcp_v4_md5_do_add(struct sock *sk, _
 	return 0;
 }
 
-EXPORT_SYMBOL(tcp_v4_md5_do_add);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_md5_do_add);
 
 static int tcp_v4_md5_add_func(struct sock *sk, struct sock *addr_sk,
 			       u8 *newkey, u8 newkeylen)
@@ -951,7 +951,7 @@ int tcp_v4_md5_do_del(struct sock *sk, _
 	return -ENOENT;
 }
 
-EXPORT_SYMBOL(tcp_v4_md5_do_del);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_md5_do_del);
 
 static void tcp_v4_clear_md5_list(struct sock *sk)
 {
@@ -1127,7 +1127,7 @@ int tcp_v4_calc_md5_hash(char *md5_hash,
 				       th, protocol, tcplen);
 }
 
-EXPORT_SYMBOL(tcp_v4_calc_md5_hash);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_calc_md5_hash);
 
 static int tcp_v4_inbound_md5_hash(struct sock *sk, struct sk_buff *skb)
 {
@@ -1936,7 +1936,7 @@ int tcp_v4_destroy_sock(struct sock *sk)
 	return 0;
 }
 
-EXPORT_SYMBOL(tcp_v4_destroy_sock);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_destroy_sock);
 
 #ifdef CONFIG_PROC_FS
 /* Proc filesystem TCP sock list dumping. */
@@ -2455,20 +2455,20 @@ void __init tcp_v4_init(struct net_proto
 		panic("Failed to create the TCP control socket.\n");
 }
 
-EXPORT_SYMBOL(ipv4_specific);
-EXPORT_SYMBOL(tcp_hashinfo);
-EXPORT_SYMBOL(tcp_prot);
-EXPORT_SYMBOL(tcp_unhash);
-EXPORT_SYMBOL(tcp_v4_conn_request);
-EXPORT_SYMBOL(tcp_v4_connect);
-EXPORT_SYMBOL(tcp_v4_do_rcv);
-EXPORT_SYMBOL(tcp_v4_remember_stamp);
-EXPORT_SYMBOL(tcp_v4_send_check);
-EXPORT_SYMBOL(tcp_v4_syn_recv_sock);
+EXPORT_SYMBOL_NS(tcp, ipv4_specific);
+EXPORT_SYMBOL_NS(tcp, tcp_hashinfo);
+EXPORT_SYMBOL_NS(tcp, tcp_prot);
+EXPORT_SYMBOL_NS(tcp, tcp_unhash);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_conn_request);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_connect);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_do_rcv);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_remember_stamp);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_send_check);
+EXPORT_SYMBOL_NS(tcp, tcp_v4_syn_recv_sock);
 
 #ifdef CONFIG_PROC_FS
-EXPORT_SYMBOL(tcp_proc_register);
-EXPORT_SYMBOL(tcp_proc_unregister);
+EXPORT_SYMBOL_NS(tcp, tcp_proc_register);
+EXPORT_SYMBOL_NS(tcp, tcp_proc_unregister);
 #endif
-EXPORT_SYMBOL(sysctl_tcp_low_latency);
+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_low_latency);
 
Index: linux/net/ipv4/tcp_minisocks.c
===================================================================
--- linux.orig/net/ipv4/tcp_minisocks.c
+++ linux/net/ipv4/tcp_minisocks.c
@@ -53,7 +53,7 @@ struct inet_timewait_death_row tcp_death
 					    (unsigned long)&tcp_death_row),
 };
 
-EXPORT_SYMBOL_GPL(tcp_death_row);
+EXPORT_SYMBOL_NS(tcp, tcp_death_row);
 
 static __inline__ int tcp_in_window(u32 seq, u32 end_seq, u32 s_win, u32 e_win)
 {
@@ -366,7 +366,7 @@ void tcp_twsk_destructor(struct sock *sk
 #endif
 }
 
-EXPORT_SYMBOL_GPL(tcp_twsk_destructor);
+EXPORT_SYMBOL_NS(tcp, tcp_twsk_destructor);
 
 static inline void TCP_ECN_openreq_child(struct tcp_sock *tp,
 					 struct request_sock *req)
@@ -734,7 +734,7 @@ int tcp_child_process(struct sock *paren
 	return ret;
 }
 
-EXPORT_SYMBOL(tcp_check_req);
-EXPORT_SYMBOL(tcp_child_process);
-EXPORT_SYMBOL(tcp_create_openreq_child);
-EXPORT_SYMBOL(tcp_timewait_state_process);
+EXPORT_SYMBOL_NS(tcp, tcp_check_req);
+EXPORT_SYMBOL_NS(tcp, tcp_child_process);
+EXPORT_SYMBOL_NS(tcp, tcp_create_openreq_child);
+EXPORT_SYMBOL_NS(tcp, tcp_timewait_state_process);
Index: linux/net/ipv4/tcp_output.c
===================================================================
--- linux.orig/net/ipv4/tcp_output.c
+++ linux/net/ipv4/tcp_output.c
@@ -2599,9 +2599,9 @@ void tcp_send_probe0(struct sock *sk)
 	}
 }
 
-EXPORT_SYMBOL(tcp_connect);
-EXPORT_SYMBOL(tcp_make_synack);
-EXPORT_SYMBOL(tcp_simple_retransmit);
-EXPORT_SYMBOL(tcp_sync_mss);
-EXPORT_SYMBOL(sysctl_tcp_tso_win_divisor);
-EXPORT_SYMBOL(tcp_mtup_init);
+EXPORT_SYMBOL_NS(tcp, tcp_connect);
+EXPORT_SYMBOL_NS(tcp, tcp_make_synack);
+EXPORT_SYMBOL_NS(tcp, tcp_simple_retransmit);
+EXPORT_SYMBOL_NS(tcp, tcp_sync_mss);
+EXPORT_SYMBOL_NS(tcpcong, sysctl_tcp_tso_win_divisor);
+EXPORT_SYMBOL_NS(tcp, tcp_mtup_init);
Index: linux/net/ipv4/tcp_timer.c
===================================================================
--- linux.orig/net/ipv4/tcp_timer.c
+++ linux/net/ipv4/tcp_timer.c
@@ -42,7 +42,7 @@ void tcp_init_xmit_timers(struct sock *s
 				  &tcp_keepalive_timer);
 }
 
-EXPORT_SYMBOL(tcp_init_xmit_timers);
+EXPORT_SYMBOL_NS(tcp, tcp_init_xmit_timers);
 
 static void tcp_write_err(struct sock *sk)
 {
Index: linux/net/ipv4/tcp_cong.c
===================================================================
--- linux.orig/net/ipv4/tcp_cong.c
+++ linux/net/ipv4/tcp_cong.c
@@ -57,7 +57,7 @@ int tcp_register_congestion_control(stru
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(tcp_register_congestion_control);
+EXPORT_SYMBOL_NS(tcpcong, tcp_register_congestion_control);
 
 /*
  * Remove congestion control algorithm, called from
@@ -71,7 +71,7 @@ void tcp_unregister_congestion_control(s
 	list_del_rcu(&ca->list);
 	spin_unlock(&tcp_cong_list_lock);
 }
-EXPORT_SYMBOL_GPL(tcp_unregister_congestion_control);
+EXPORT_SYMBOL_NS(tcpcong, tcp_unregister_congestion_control);
 
 /* Assign choice of congestion control. */
 void tcp_init_congestion_control(struct sock *sk)
@@ -315,7 +315,7 @@ void tcp_slow_start(struct tcp_sock *tp)
 			tp->snd_cwnd++;
 	}
 }
-EXPORT_SYMBOL_GPL(tcp_slow_start);
+EXPORT_SYMBOL_NS(tcpcong, tcp_slow_start);
 
 /*
  * TCP Reno congestion control
@@ -355,7 +355,7 @@ void tcp_reno_cong_avoid(struct sock *sk
 			tp->snd_cwnd_cnt++;
 	}
 }
-EXPORT_SYMBOL_GPL(tcp_reno_cong_avoid);
+EXPORT_SYMBOL_NS(tcpcong, tcp_reno_cong_avoid);
 
 /* Slow start threshold is half the congestion window (min 2) */
 u32 tcp_reno_ssthresh(struct sock *sk)
@@ -363,7 +363,7 @@ u32 tcp_reno_ssthresh(struct sock *sk)
 	const struct tcp_sock *tp = tcp_sk(sk);
 	return max(tp->snd_cwnd >> 1U, 2U);
 }
-EXPORT_SYMBOL_GPL(tcp_reno_ssthresh);
+EXPORT_SYMBOL_NS(tcpcong, tcp_reno_ssthresh);
 
 /* Lower bound on congestion window with halving. */
 u32 tcp_reno_min_cwnd(const struct sock *sk)
@@ -371,7 +371,7 @@ u32 tcp_reno_min_cwnd(const struct sock 
 	const struct tcp_sock *tp = tcp_sk(sk);
 	return tp->snd_ssthresh/2;
 }
-EXPORT_SYMBOL_GPL(tcp_reno_min_cwnd);
+EXPORT_SYMBOL_NS(tcpcong, tcp_reno_min_cwnd);
 
 struct tcp_congestion_ops tcp_reno = {
 	.flags		= TCP_CONG_NON_RESTRICTED,
@@ -393,4 +393,4 @@ struct tcp_congestion_ops tcp_init_conge
 	.cong_avoid	= tcp_reno_cong_avoid,
 	.min_cwnd	= tcp_reno_min_cwnd,
 };
-EXPORT_SYMBOL_GPL(tcp_init_congestion_ops);
+EXPORT_SYMBOL_NS(tcpcong, tcp_init_congestion_ops);
Index: linux/net/ipv4/tcp_input.c
===================================================================
--- linux.orig/net/ipv4/tcp_input.c
+++ linux/net/ipv4/tcp_input.c
@@ -5163,9 +5163,9 @@ discard:
 	return 0;
 }
 
-EXPORT_SYMBOL(sysctl_tcp_ecn);
-EXPORT_SYMBOL(sysctl_tcp_reordering);
-EXPORT_SYMBOL(tcp_parse_options);
-EXPORT_SYMBOL(tcp_rcv_established);
-EXPORT_SYMBOL(tcp_rcv_state_process);
-EXPORT_SYMBOL(tcp_initialize_rcv_mss);
+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_ecn);
+EXPORT_SYMBOL_NS(tcp, sysctl_tcp_reordering);
+EXPORT_SYMBOL_NS(tcp, tcp_parse_options);
+EXPORT_SYMBOL_NS(tcp, tcp_rcv_established);
+EXPORT_SYMBOL_NS(tcp, tcp_rcv_state_process);
+EXPORT_SYMBOL_NS(tcp, tcp_initialize_rcv_mss);

[PATCH RFC] [8/9] Put UDP exports into a namespace

[Andi Kleen]

The UDP exports are only used by UDPv6 and UDP lite. They are internal functions
not supposed to be used by anybody else. So turn them into a name space that 
only allows those.

---
 net/ipv4/udp.c     |   27 +++++++++++++++------------
 net/ipv4/udplite.c |    6 +++---
 2 files changed, 18 insertions(+), 15 deletions(-)

Index: linux/net/ipv4/udp.c
===================================================================
--- linux.orig/net/ipv4/udp.c
+++ linux/net/ipv4/udp.c
@@ -105,6 +105,9 @@
 #include <net/xfrm.h>
 #include "udp_impl.h"
 
+MODULE_NAMESPACE_ALLOW(udp, udplite);
+MODULE_NAMESPACE_ALLOW(udp, ipv6);
+
 /*
  *	Snmp MIB for the UDP layer
  */
@@ -1641,18 +1644,18 @@ void udp4_proc_exit(void)
 }
 #endif /* CONFIG_PROC_FS */
 
-EXPORT_SYMBOL(udp_disconnect);
-EXPORT_SYMBOL(udp_hash);
-EXPORT_SYMBOL(udp_hash_lock);
-EXPORT_SYMBOL(udp_ioctl);
-EXPORT_SYMBOL(udp_get_port);
-EXPORT_SYMBOL(udp_prot);
-EXPORT_SYMBOL(udp_sendmsg);
-EXPORT_SYMBOL(udp_lib_getsockopt);
-EXPORT_SYMBOL(udp_lib_setsockopt);
-EXPORT_SYMBOL(udp_poll);
+EXPORT_SYMBOL_NS(udp, udp_disconnect);
+EXPORT_SYMBOL_NS(udp, udp_hash);
+EXPORT_SYMBOL_NS(udp, udp_hash_lock);
+EXPORT_SYMBOL_NS(udp, udp_ioctl);
+EXPORT_SYMBOL_NS(udp, udp_get_port);
+EXPORT_SYMBOL_NS(udp, udp_prot);
+EXPORT_SYMBOL_NS(udp, udp_sendmsg);
+EXPORT_SYMBOL_NS(udp, udp_lib_getsockopt);
+EXPORT_SYMBOL_NS(udp, udp_lib_setsockopt);
+EXPORT_SYMBOL_NS(udp, udp_poll);
 
 #ifdef CONFIG_PROC_FS
-EXPORT_SYMBOL(udp_proc_register);
-EXPORT_SYMBOL(udp_proc_unregister);
+EXPORT_SYMBOL_NS(udp, udp_proc_register);
+EXPORT_SYMBOL_NS(udp, udp_proc_unregister);
 #endif
Index: linux/net/ipv4/udplite.c
===================================================================
--- linux.orig/net/ipv4/udplite.c
+++ linux/net/ipv4/udplite.c
@@ -113,6 +113,6 @@ out_register_err:
 	printk(KERN_CRIT "%s: Cannot add UDP-Lite protocol.\n", __FUNCTION__);
 }
 
-EXPORT_SYMBOL(udplite_hash);
-EXPORT_SYMBOL(udplite_prot);
-EXPORT_SYMBOL(udplite_get_port);
+EXPORT_SYMBOL_NS(udp, udplite_hash);
+EXPORT_SYMBOL_NS(udp, udplite_prot);
+EXPORT_SYMBOL_NS(udp, udplite_get_port);

[PATCH RFC] [9/9] Add a inet namespace

[Andi Kleen]

Shared by IP, IPv6, DCCP, UDPLITE, SCTP. 

The symbols used by tunnel modules weren't put into any name space
because there are quite a lot of them.

---
 net/core/fib_rules.c            |    9 ++++--
 net/ipv4/af_inet.c              |   52 ++++++++++++++++++++++++----------------
 net/ipv4/arp.c                  |    1 
 net/ipv4/icmp.c                 |   10 +++----
 net/ipv4/inet_connection_sock.c |   40 +++++++++++++++---------------
 net/ipv4/inet_diag.c            |    4 +--
 net/ipv4/inet_hashtables.c      |    8 +++---
 net/ipv4/inet_timewait_sock.c   |   12 ++++-----
 net/ipv4/ip_input.c             |    2 -
 net/ipv4/ip_output.c            |    7 +++--
 net/ipv4/ip_sockglue.c          |   10 +++----
 11 files changed, 86 insertions(+), 69 deletions(-)

Index: linux/net/ipv4/af_inet.c
===================================================================
--- linux.orig/net/ipv4/af_inet.c
+++ linux/net/ipv4/af_inet.c
@@ -218,7 +218,7 @@ out:
 }
 
 u32 inet_ehash_secret __read_mostly;
-EXPORT_SYMBOL(inet_ehash_secret);
+EXPORT_SYMBOL_NS(inet, inet_ehash_secret);
 
 /*
  * inet_ehash_secret must be set exactly once
@@ -235,7 +235,7 @@ void build_ehash_secret(void)
 		inet_ehash_secret = rnd;
 	spin_unlock_bh(&inetsw_lock);
 }
-EXPORT_SYMBOL(build_ehash_secret);
+EXPORT_SYMBOL_NS(inet, build_ehash_secret);
 
 /*
  *	Create an inet socket.
@@ -1127,7 +1127,7 @@ int inet_sk_rebuild_header(struct sock *
 	return err;
 }
 
-EXPORT_SYMBOL(inet_sk_rebuild_header);
+EXPORT_SYMBOL_NS(inet,inet_sk_rebuild_header);
 
 static int inet_gso_send_check(struct sk_buff *skb)
 {
@@ -1235,6 +1235,8 @@ unsigned long snmp_fold_field(void *mib[
 	}
 	return res;
 }
+/* AK: Not in inet namespace because they're a generic facility. Probably
+   should be in another file though. */
 EXPORT_SYMBOL_GPL(snmp_fold_field);
 
 int snmp_mib_init(void *ptr[2], size_t mibsize, size_t mibalign)
@@ -1499,20 +1501,30 @@ static int __init ipv4_proc_init(void)
 
 MODULE_ALIAS_NETPROTO(PF_INET);
 
-EXPORT_SYMBOL(inet_accept);
-EXPORT_SYMBOL(inet_bind);
-EXPORT_SYMBOL(inet_dgram_connect);
-EXPORT_SYMBOL(inet_dgram_ops);
-EXPORT_SYMBOL(inet_getname);
-EXPORT_SYMBOL(inet_ioctl);
-EXPORT_SYMBOL(inet_listen);
-EXPORT_SYMBOL(inet_register_protosw);
-EXPORT_SYMBOL(inet_release);
-EXPORT_SYMBOL(inet_sendmsg);
-EXPORT_SYMBOL(inet_shutdown);
-EXPORT_SYMBOL(inet_sock_destruct);
-EXPORT_SYMBOL(inet_stream_connect);
-EXPORT_SYMBOL(inet_stream_ops);
-EXPORT_SYMBOL(inet_unregister_protosw);
-EXPORT_SYMBOL(net_statistics);
-EXPORT_SYMBOL(sysctl_ip_nonlocal_bind);
+MODULE_NAMESPACE_ALLOW(inet, ipv6);
+MODULE_NAMESPACE_ALLOW(inet, udplite);
+MODULE_NAMESPACE_ALLOW(inet, dccp_ipv6);
+MODULE_NAMESPACE_ALLOW(inet, dccp_ipv4);
+MODULE_NAMESPACE_ALLOW(inet, dccp);
+MODULE_NAMESPACE_ALLOW(inet, sctp);
+
+/* RED-PEN: would be better to fix wanrouter */
+MODULE_NAMESPACE_ALLOW(inet, wanrouter);
+
+EXPORT_SYMBOL_NS(inet,inet_accept);
+EXPORT_SYMBOL_NS(inet,inet_bind);
+EXPORT_SYMBOL_NS(inet,inet_dgram_connect);
+EXPORT_SYMBOL_NS(inet,inet_dgram_ops);
+EXPORT_SYMBOL_NS(inet,inet_getname);
+EXPORT_SYMBOL_NS(inet,inet_ioctl);
+EXPORT_SYMBOL_NS(inet,inet_listen);
+EXPORT_SYMBOL_NS(inet,inet_register_protosw);
+EXPORT_SYMBOL_NS(inet,inet_release);
+EXPORT_SYMBOL_NS(inet,inet_sendmsg);
+EXPORT_SYMBOL_NS(inet,inet_shutdown);
+EXPORT_SYMBOL_NS(inet,inet_sock_destruct);
+EXPORT_SYMBOL_NS(inet,inet_stream_connect);
+EXPORT_SYMBOL_NS(inet,inet_stream_ops);
+EXPORT_SYMBOL_NS(inet,inet_unregister_protosw);
+EXPORT_SYMBOL_NS(inet,net_statistics);
+EXPORT_SYMBOL_NS(inet,sysctl_ip_nonlocal_bind);
Index: linux/net/ipv4/arp.c
===================================================================
--- linux.orig/net/ipv4/arp.c
+++ linux/net/ipv4/arp.c
@@ -1406,6 +1406,7 @@ static int __init arp_proc_init(void)
 
 #endif /* CONFIG_PROC_FS */
 
+/* No namespace because those are used by various drivers */
 EXPORT_SYMBOL(arp_broken_ops);
 EXPORT_SYMBOL(arp_find);
 EXPORT_SYMBOL(arp_create);
Index: linux/net/ipv4/icmp.c
===================================================================
--- linux.orig/net/ipv4/icmp.c
+++ linux/net/ipv4/icmp.c
@@ -1101,7 +1101,7 @@ void __init icmp_init(struct net_proto_f
 	}
 }
 
-EXPORT_SYMBOL(icmp_err_convert);
-EXPORT_SYMBOL(icmp_send);
-EXPORT_SYMBOL(icmp_statistics);
-EXPORT_SYMBOL(xrlim_allow);
+EXPORT_SYMBOL_NS(inet, icmp_err_convert);
+EXPORT_SYMBOL_NS(inet, icmp_send);
+EXPORT_SYMBOL_NS(inet, icmp_statistics);
+EXPORT_SYMBOL_NS(inet, xrlim_allow);
Index: linux/net/ipv4/inet_connection_sock.c
===================================================================
--- linux.orig/net/ipv4/inet_connection_sock.c
+++ linux/net/ipv4/inet_connection_sock.c
@@ -26,7 +26,7 @@
 
 #ifdef INET_CSK_DEBUG
 const char inet_csk_timer_bug_msg[] = "inet_csk BUG: unknown timer value\n";
-EXPORT_SYMBOL(inet_csk_timer_bug_msg);
+EXPORT_SYMBOL_NS(inet, inet_csk_timer_bug_msg);
 #endif
 
 /*
@@ -73,7 +73,7 @@ int inet_csk_bind_conflict(const struct 
 	return node != NULL;
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_bind_conflict);
+EXPORT_SYMBOL_NS(inet, inet_csk_bind_conflict);
 
 /* Obtain a reference to a local port for the given sock,
  * if snum is zero it means select any available local port.
@@ -170,7 +170,7 @@ fail:
 	return ret;
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_get_port);
+EXPORT_SYMBOL_NS(inet, inet_csk_get_port);
 
 /*
  * Wait for an incoming connection, avoid race conditions. This must be called
@@ -263,7 +263,7 @@ out_err:
 	goto out;
 }
 
-EXPORT_SYMBOL(inet_csk_accept);
+EXPORT_SYMBOL_NS(inet, inet_csk_accept);
 
 /*
  * Using different timers for retransmit, delayed acks and probes
@@ -292,7 +292,7 @@ void inet_csk_init_xmit_timers(struct so
 	icsk->icsk_pending = icsk->icsk_ack.pending = 0;
 }
 
-EXPORT_SYMBOL(inet_csk_init_xmit_timers);
+EXPORT_SYMBOL_NS(inet, inet_csk_init_xmit_timers);
 
 void inet_csk_clear_xmit_timers(struct sock *sk)
 {
@@ -305,21 +305,21 @@ void inet_csk_clear_xmit_timers(struct s
 	sk_stop_timer(sk, &sk->sk_timer);
 }
 
-EXPORT_SYMBOL(inet_csk_clear_xmit_timers);
+EXPORT_SYMBOL_NS(inet, inet_csk_clear_xmit_timers);
 
 void inet_csk_delete_keepalive_timer(struct sock *sk)
 {
 	sk_stop_timer(sk, &sk->sk_timer);
 }
 
-EXPORT_SYMBOL(inet_csk_delete_keepalive_timer);
+EXPORT_SYMBOL_NS(inet, inet_csk_delete_keepalive_timer);
 
 void inet_csk_reset_keepalive_timer(struct sock *sk, unsigned long len)
 {
 	sk_reset_timer(sk, &sk->sk_timer, jiffies + len);
 }
 
-EXPORT_SYMBOL(inet_csk_reset_keepalive_timer);
+EXPORT_SYMBOL_NS(inet, inet_csk_reset_keepalive_timer);
 
 struct dst_entry* inet_csk_route_req(struct sock *sk,
 				     const struct request_sock *req)
@@ -352,7 +352,7 @@ struct dst_entry* inet_csk_route_req(str
 	return &rt->u.dst;
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_route_req);
+EXPORT_SYMBOL_NS(inet, inet_csk_route_req);
 
 static inline u32 inet_synq_hash(const __be32 raddr, const __be16 rport,
 				 const u32 rnd, const u32 synq_hsize)
@@ -394,7 +394,7 @@ struct request_sock *inet_csk_search_req
 	return req;
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_search_req);
+EXPORT_SYMBOL_NS(inet, inet_csk_search_req);
 
 void inet_csk_reqsk_queue_hash_add(struct sock *sk, struct request_sock *req,
 				   unsigned long timeout)
@@ -411,7 +411,7 @@ void inet_csk_reqsk_queue_hash_add(struc
 /* Only thing we need from tcp.h */
 extern int sysctl_tcp_synack_retries;
 
-EXPORT_SYMBOL_GPL(inet_csk_reqsk_queue_hash_add);
+EXPORT_SYMBOL_NS(inet, inet_csk_reqsk_queue_hash_add);
 
 void inet_csk_reqsk_queue_prune(struct sock *parent,
 				const unsigned long interval,
@@ -500,7 +500,7 @@ void inet_csk_reqsk_queue_prune(struct s
 		inet_csk_reset_keepalive_timer(parent, interval);
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_reqsk_queue_prune);
+EXPORT_SYMBOL_NS(inet, inet_csk_reqsk_queue_prune);
 
 struct sock *inet_csk_clone(struct sock *sk, const struct request_sock *req,
 			    const gfp_t priority)
@@ -528,7 +528,7 @@ struct sock *inet_csk_clone(struct sock 
 	return newsk;
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_clone);
+EXPORT_SYMBOL_NS(inet, inet_csk_clone);
 
 /*
  * At this point, there should be no process reference to this
@@ -559,7 +559,7 @@ void inet_csk_destroy_sock(struct sock *
 	sock_put(sk);
 }
 
-EXPORT_SYMBOL(inet_csk_destroy_sock);
+EXPORT_SYMBOL_NS(inet, inet_csk_destroy_sock);
 
 int inet_csk_listen_start(struct sock *sk, const int nr_table_entries)
 {
@@ -594,7 +594,7 @@ int inet_csk_listen_start(struct sock *s
 	return -EADDRINUSE;
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_listen_start);
+EXPORT_SYMBOL_NS(inet, inet_csk_listen_start);
 
 /*
  *	This routine closes sockets which have been at least partially
@@ -649,7 +649,7 @@ void inet_csk_listen_stop(struct sock *s
 	BUG_TRAP(!sk->sk_ack_backlog);
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_listen_stop);
+EXPORT_SYMBOL_NS(inet, inet_csk_listen_stop);
 
 void inet_csk_addr2sockaddr(struct sock *sk, struct sockaddr *uaddr)
 {
@@ -661,7 +661,7 @@ void inet_csk_addr2sockaddr(struct sock 
 	sin->sin_port		= inet->dport;
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_addr2sockaddr);
+EXPORT_SYMBOL_NS(inet, inet_csk_addr2sockaddr);
 
 int inet_csk_ctl_sock_create(struct socket **sock, unsigned short family,
 			     unsigned short type, unsigned char protocol)
@@ -680,7 +680,7 @@ int inet_csk_ctl_sock_create(struct sock
 	return rc;
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_ctl_sock_create);
+EXPORT_SYMBOL_NS(inet, inet_csk_ctl_sock_create);
 
 #ifdef CONFIG_COMPAT
 int inet_csk_compat_getsockopt(struct sock *sk, int level, int optname,
@@ -695,7 +695,7 @@ int inet_csk_compat_getsockopt(struct so
 					     optval, optlen);
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_compat_getsockopt);
+EXPORT_SYMBOL_NS(inet, inet_csk_compat_getsockopt);
 
 int inet_csk_compat_setsockopt(struct sock *sk, int level, int optname,
 			       char __user *optval, int optlen)
@@ -709,5 +709,5 @@ int inet_csk_compat_setsockopt(struct so
 					     optval, optlen);
 }
 
-EXPORT_SYMBOL_GPL(inet_csk_compat_setsockopt);
+EXPORT_SYMBOL_NS(inet, inet_csk_compat_setsockopt);
 #endif
Index: linux/net/ipv4/ip_output.c
===================================================================
--- linux.orig/net/ipv4/ip_output.c
+++ linux/net/ipv4/ip_output.c
@@ -669,7 +669,7 @@ fail:
 	return err;
 }
 
-EXPORT_SYMBOL(ip_fragment);
+EXPORT_SYMBOL_NS(inet,ip_fragment);
 
 int
 ip_generic_getfrag(void *from, char *to, int offset, int len, int odd, struct sk_buff *skb)
@@ -1409,6 +1409,7 @@ void __init ip_init(void)
 #endif
 }
 
-EXPORT_SYMBOL(ip_generic_getfrag);
-EXPORT_SYMBOL(ip_queue_xmit);
+EXPORT_SYMBOL_NS(inet,ip_generic_getfrag);
+EXPORT_SYMBOL_NS(inet,ip_queue_xmit);
+/* ip_send_check is widely used by netfilter modules */
 EXPORT_SYMBOL(ip_send_check);
Index: linux/net/ipv4/inet_diag.c
===================================================================
--- linux.orig/net/ipv4/inet_diag.c
+++ linux/net/ipv4/inet_diag.c
@@ -873,7 +873,7 @@ int inet_diag_register(const struct inet
 out:
 	return err;
 }
-EXPORT_SYMBOL_GPL(inet_diag_register);
+EXPORT_SYMBOL_NS(inet, inet_diag_register);
 
 void inet_diag_unregister(const struct inet_diag_handler *h)
 {
@@ -888,7 +888,7 @@ void inet_diag_unregister(const struct i
 
 	synchronize_rcu();
 }
-EXPORT_SYMBOL_GPL(inet_diag_unregister);
+EXPORT_SYMBOL_NS(inet, inet_diag_unregister);
 
 static int __init inet_diag_init(void)
 {
Index: linux/net/ipv4/inet_hashtables.c
===================================================================
--- linux.orig/net/ipv4/inet_hashtables.c
+++ linux/net/ipv4/inet_hashtables.c
@@ -86,7 +86,7 @@ void inet_put_port(struct inet_hashinfo 
 	local_bh_enable();
 }
 
-EXPORT_SYMBOL(inet_put_port);
+EXPORT_SYMBOL_NS(inet, inet_put_port);
 
 /*
  * This lock without WQ_FLAG_EXCLUSIVE is good on UP and it can be very bad on SMP.
@@ -116,7 +116,7 @@ void inet_listen_wlock(struct inet_hashi
 	}
 }
 
-EXPORT_SYMBOL(inet_listen_wlock);
+EXPORT_SYMBOL_NS(inet, inet_listen_wlock);
 
 /*
  * Don't inline this cruft. Here are some nice properties to exploit here. The
@@ -188,7 +188,7 @@ sherry_cache:
 	read_unlock(&hashinfo->lhash_lock);
 	return sk;
 }
-EXPORT_SYMBOL_GPL(__inet_lookup_listener);
+EXPORT_SYMBOL_NS(inet, __inet_lookup_listener);
 
 /* called with local bh disabled */
 static int __inet_check_established(struct inet_timewait_death_row *death_row,
@@ -364,4 +364,4 @@ out:
 	}
 }
 
-EXPORT_SYMBOL_GPL(inet_hash_connect);
+EXPORT_SYMBOL_NS(inet, inet_hash_connect);
Index: linux/net/ipv4/ip_input.c
===================================================================
--- linux.orig/net/ipv4/ip_input.c
+++ linux/net/ipv4/ip_input.c
@@ -453,4 +453,4 @@ out:
 	return NET_RX_DROP;
 }
 
-EXPORT_SYMBOL(ip_statistics);
+EXPORT_SYMBOL_NS(inet, ip_statistics);
Index: linux/net/core/fib_rules.c
===================================================================
--- linux.orig/net/core/fib_rules.c
+++ linux/net/core/fib_rules.c
@@ -100,7 +100,7 @@ errout:
 	return err;
 }
 
-EXPORT_SYMBOL_GPL(fib_rules_register);
+EXPORT_SYMBOL_NS(fib, fib_rules_register);
 
 static void cleanup_ops(struct fib_rules_ops *ops)
 {
@@ -135,7 +135,7 @@ out:
 	return err;
 }
 
-EXPORT_SYMBOL_GPL(fib_rules_unregister);
+EXPORT_SYMBOL_NS(fib, fib_rules_unregister);
 
 static int fib_rule_match(struct fib_rule *rule, struct fib_rules_ops *ops,
 			  struct flowi *fl, int flags)
@@ -195,7 +195,7 @@ out:
 	return err;
 }
 
-EXPORT_SYMBOL_GPL(fib_rules_lookup);
+EXPORT_SYMBOL_NS(fib, fib_rules_lookup);
 
 static int validate_rulemsg(struct fib_rule_hdr *frh, struct nlattr **tb,
 			    struct fib_rules_ops *ops)
@@ -658,3 +658,6 @@ static int __init fib_rules_init(void)
 }
 
 subsys_initcall(fib_rules_init);
+
+MODULE_NAMESPACE_ALLOW(fib, ipv6);
+MODULE_NAMESPACE_ALLOW(fib, decnet);
Index: linux/net/ipv4/inet_timewait_sock.c
===================================================================
--- linux.orig/net/ipv4/inet_timewait_sock.c
+++ linux/net/ipv4/inet_timewait_sock.c
@@ -85,7 +85,7 @@ void __inet_twsk_hashdance(struct inet_t
 	write_unlock(lock);
 }
 
-EXPORT_SYMBOL_GPL(__inet_twsk_hashdance);
+EXPORT_SYMBOL_NS(inet, __inet_twsk_hashdance);
 
 struct inet_timewait_sock *inet_twsk_alloc(const struct sock *sk, const int state)
 {
@@ -117,7 +117,7 @@ struct inet_timewait_sock *inet_twsk_all
 	return tw;
 }
 
-EXPORT_SYMBOL_GPL(inet_twsk_alloc);
+EXPORT_SYMBOL_NS(inet, inet_twsk_alloc);
 
 /* Returns non-zero if quota exceeded.  */
 static int inet_twdr_do_twkill_work(struct inet_timewait_death_row *twdr,
@@ -192,7 +192,7 @@ out:
 	spin_unlock(&twdr->death_lock);
 }
 
-EXPORT_SYMBOL_GPL(inet_twdr_hangman);
+EXPORT_SYMBOL_NS(inet, inet_twdr_hangman);
 
 extern void twkill_slots_invalid(void);
 
@@ -225,7 +225,7 @@ void inet_twdr_twkill_work(struct work_s
 	}
 }
 
-EXPORT_SYMBOL_GPL(inet_twdr_twkill_work);
+EXPORT_SYMBOL_NS(inet, inet_twdr_twkill_work);
 
 /* These are always called from BH context.  See callers in
  * tcp_input.c to verify this.
@@ -326,7 +326,7 @@ void inet_twsk_schedule(struct inet_time
 	spin_unlock(&twdr->death_lock);
 }
 
-EXPORT_SYMBOL_GPL(inet_twsk_schedule);
+EXPORT_SYMBOL_NS(inet, inet_twsk_schedule);
 
 void inet_twdr_twcal_tick(unsigned long data)
 {
@@ -382,4 +382,4 @@ out:
 	spin_unlock(&twdr->death_lock);
 }
 
-EXPORT_SYMBOL_GPL(inet_twdr_twcal_tick);
+EXPORT_SYMBOL_NS(inet, inet_twdr_twcal_tick);
Index: linux/net/ipv4/ip_sockglue.c
===================================================================
--- linux.orig/net/ipv4/ip_sockglue.c
+++ linux/net/ipv4/ip_sockglue.c
@@ -943,7 +943,7 @@ int compat_ip_setsockopt(struct sock *sk
 	return err;
 }
 
-EXPORT_SYMBOL(compat_ip_setsockopt);
+EXPORT_SYMBOL_NS(inet, compat_ip_setsockopt);
 #endif
 
 /*
@@ -1206,10 +1206,10 @@ int compat_ip_getsockopt(struct sock *sk
 	return err;
 }
 
-EXPORT_SYMBOL(compat_ip_getsockopt);
+EXPORT_SYMBOL_NS(inet, compat_ip_getsockopt);
 #endif
 
-EXPORT_SYMBOL(ip_cmsg_recv);
+EXPORT_SYMBOL_NS(inet, ip_cmsg_recv);
 
-EXPORT_SYMBOL(ip_getsockopt);
-EXPORT_SYMBOL(ip_setsockopt);
+EXPORT_SYMBOL_NS(inet, ip_getsockopt);
+EXPORT_SYMBOL_NS(inet, ip_setsockopt);

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Arjan van de Ven]

On Thu, 22 Nov 2007 03:43:06 +0100 (CET)
Andi Kleen <ak@suse.de> wrote:

> 
> There seems to be rough consensus that the kernel currently has too
> many exported symbols. A lot of these exports are generally usable
> utility functions or important driver interfaces; but another large
> part are functions intended by only one or two very specific modules
> for a very specific purpose. One example is the TCP code. It has most
> of its internals exported, but only for use by tcp_ipv6.c (and now a
> few more by the TCP/IP congestion modules) But it doesn't make sense
> to include these exported for a specific module functions into a
> broader "kernel interface".   External modules assume they can use
> these functions, but they were never intended for that.
> 
> This patch allows to export symbols only for specific modules by 
> introducing symbol name spaces. A module name space has a white
> list of modules that are allowed to import symbols for it; all others
> can't use the symbols.
> 
> It adds two new macros: 
> 
> MODULE_NAMESPACE_ALLOW(namespace, module);
> 
> Allow module to import symbols from namespace. module is the module
> name without .ko as displayed by lsmod.  Must be in the same module
> as the export (and be duplicated if there are multiple modules
> exporting symbols to a namespace).  Multiple allows for the same name
> space are allowed.
> 
> EXPORT_SYMBOL_NS(namespace, symbol);
> 

Hi,

I like this concept in general; I have one minor comment; right now
your namespace argument is like

EXPORT_SYMBOL_NS(foo, some_symbol);

from a language-like pov I kinda wonder if it's nicer to do

EXPORT_SYMBOL_NS("foo", some_symbol);

because foo isn't something in C scope, but more a string-like
identifier...



-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

> I like this concept in general; I have one minor comment; right now
> your namespace argument is like
> 
> EXPORT_SYMBOL_NS(foo, some_symbol);
> 
> from a language-like pov I kinda wonder if it's nicer to do
> 
> EXPORT_SYMBOL_NS("foo", some_symbol);
> 
> because foo isn't something in C scope, but more a string-like
> identifier...

That wouldn't work for MODULE_ALLOW() because it appends the namespace
to other identifiers. I don't know of a way in the C processor to get
back from a string to a ## concatenable identifier.

For EXPORT_SYMBOL_NS it would be in theory possible, but making 
it asymmetric to MODULE_ALLOW would be ugly imho.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Dave Jones]

On Thu, Nov 22, 2007 at 03:43:06AM +0100, Andi Kleen wrote:

 > There seems to be rough consensus that the kernel currently has too many 
 > exported symbols. A lot of these exports are generally usable utility 
 > functions or important driver interfaces; but another large part are functions
 > intended by only one or two very specific modules for a very specific purpose.
 > One example is the TCP code. It has most of its internals exported, but 
 > only for use by tcp_ipv6.c (and now a few more by the TCP/IP congestion modules) 
 > But it doesn't make sense to include these exported for a specific module
 > functions into a broader "kernel interface".   External modules assume
 > they can use these functions, but they were never intended for that.
 > 
 > This patch allows to export symbols only for specific modules by 
 > introducing symbol name spaces. A module name space has a white
 > list of modules that are allowed to import symbols for it; all others
 > can't use the symbols.

I really like this patchset.   Definitely a step in the right direction imo.
Looks like some nits there that checkpatch will probably pick up on,
but otherwise, looks very straightforward too.

Kudos.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Thursday 22 November 2007 13:43:06 Andi Kleen wrote:
> There seems to be rough consensus that the kernel currently has too many
> exported symbols. A lot of these exports are generally usable utility
> functions or important driver interfaces; but another large part are
> functions intended by only one or two very specific modules for a very
> specific purpose.

Hi Andi,

    This is an interesting idea, thanks for the code!  My only question is 
whether we can get most of this benefit by dropping the indirection of 
namespaces and have something like "EXPORT_SYMBOL_TO(sym, modname)"?  It
doesn't work so well for exporting to a group of modules, but that seems
a reasonable line to draw anyway.

Cheers,
Rusty.
PS.  Probably better to use the standard warnx and errx in modpost, too.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Dave Young]

On Nov 22, 2007 11:56 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Thursday 22 November 2007 13:43:06 Andi Kleen wrote:
> > There seems to be rough consensus that the kernel currently has too many
> > exported symbols. A lot of these exports are generally usable utility
> > functions or important driver interfaces; but another large part are
> > functions intended by only one or two very specific modules for a very
> > specific purpose.
>
> Hi Andi,
>
>     This is an interesting idea, thanks for the code!  My only question is
> whether we can get most of this benefit by dropping the indirection of
> namespaces and have something like "EXPORT_SYMBOL_TO(sym, modname)"?

Andy, I like your idea.  IMHO, as Rusty said a simple EXPORT_SYMBOL_TO
is better.

And I wonder if it is possible to export to something like  the struct
device_driver? If it's possible then it will not limited to modules.

> doesn't work so well for exporting to a group of modules, but that seems
> a reasonable line to draw anyway.
>
> Cheers,
> Rusty.
> PS.  Probably better to use the standard warnx and errx in modpost, too.
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Christoph Hellwig]

On Thu, Nov 22, 2007 at 02:56:22PM +1100, Rusty Russell wrote:
>     This is an interesting idea, thanks for the code!  My only question is 
> whether we can get most of this benefit by dropping the indirection of 
> namespaces and have something like "EXPORT_SYMBOL_TO(sym, modname)"?  It
> doesn't work so well for exporting to a group of modules, but that seems
> a reasonable line to draw anyway.

I'd say exporting to a group of modules is the main use case.  E.g. in
scsi there would be symbols exported to transport class modules only
or lots of the vfs_ symbols would be exported only to stackable filesystems
or nfsd.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Christoph Hellwig]

Very nice, looking forward to organize the exports mess a bit more.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Thursday 22 November 2007 04:56, Rusty Russell wrote:

>     This is an interesting idea, thanks for the code!  My only question is
> whether we can get most of this benefit by dropping the indirection of
> namespaces and have something like "EXPORT_SYMBOL_TO(sym, modname)"?  It
> doesn't work so well for exporting to a group of modules, but that seems
> a reasonable line to draw anyway.

That would explode quickly already even for my example "inet" namespace.
It already has several modules.  I don't think so much duplication would be a 
good idea.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Thursday 22 November 2007 12:06, Christoph Hellwig wrote:
> Very nice, looking forward to organize the exports mess a bit more.

I would need people to help me converting more subsystems to this new scheme.

In particular all exports that are only used by a single module are direct 
candidates for a namespace. For the others it has to be checked by someone
who knows the particular subsystem well.

e.g. for SCSI it would be nice to put the symbols only used by sd/sr/sg etc.
into a namespace.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Arnaldo Carvalho de Melo]

Em Thu, Nov 22, 2007 at 03:43:06AM +0100, Andi Kleen escreveu:
> 
> There seems to be rough consensus that the kernel currently has too many 
> exported symbols. A lot of these exports are generally usable utility 
> functions or important driver interfaces; but another large part are functions
> intended by only one or two very specific modules for a very specific purpose.
> One example is the TCP code. It has most of its internals exported, but 
> only for use by tcp_ipv6.c (and now a few more by the TCP/IP congestion modules) 
> But it doesn't make sense to include these exported for a specific module
> functions into a broader "kernel interface".   External modules assume
> they can use these functions, but they were never intended for that.

Thank you for doing this.

Creating the DCCP and its congestion control infrastructure (CCID)
module namespaces is now on my TODO list. :-)

- Arnaldo

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Christoph Hellwig]

On Thu, Nov 22, 2007 at 12:54:49PM +0100, Andi Kleen wrote:
> On Thursday 22 November 2007 12:06, Christoph Hellwig wrote:
> > Very nice, looking forward to organize the exports mess a bit more.
> 
> I would need people to help me converting more subsystems to this new scheme.
> 
> In particular all exports that are only used by a single module are direct 
> candidates for a namespace. For the others it has to be checked by someone
> who knows the particular subsystem well.
> 
> e.g. for SCSI it would be nice to put the symbols only used by sd/sr/sg etc.
> into a namespace.

True.  I'll take care of the scsi bits once this goes in.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

> Creating the DCCP and its congestion control infrastructure (CCID)
> module namespaces is now on my TODO list. :-)

My original patchkit had DCCP actually done, but I ran into some problem
while forward porting and disabled it again. But should be reasonably
easy to resurrect.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

> Andy, I like your idea.  IMHO, as Rusty said a simple EXPORT_SYMBOL_TO
> is better.

I don't think so. e.g. tcpcong would be very very messy this way.

> And I wonder if it is possible to export to something like  the struct
> device_driver? If it's possible then it will not limited to modules.

Not sure I follow you. Can you expand? 

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Thursday 22 November 2007 22:05:45 Christoph Hellwig wrote:
> On Thu, Nov 22, 2007 at 02:56:22PM +1100, Rusty Russell wrote:
> >     This is an interesting idea, thanks for the code!  My only question
> > is whether we can get most of this benefit by dropping the indirection of
> > namespaces and have something like "EXPORT_SYMBOL_TO(sym, modname)"?  It
> > doesn't work so well for exporting to a group of modules, but that seems
> > a reasonable line to draw anyway.
>
> I'd say exporting to a group of modules is the main use case.  E.g. in
> scsi there would be symbols exported to transport class modules only
> or lots of the vfs_ symbols would be exported only to stackable filesystems
> or nfsd.

That's my point.  If there's a whole class of modules which can use a symbol, 
why are we ruling out external modules?  If that's what you want, why not 
have a list of permitted modules compiled into the kernel and allow no 
others?

Cheers,
Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Thursday 22 November 2007 22:46:23 Andi Kleen wrote:
> On Thursday 22 November 2007 04:56, Rusty Russell wrote:
> >     This is an interesting idea, thanks for the code!  My only question
> > is whether we can get most of this benefit by dropping the indirection of
> > namespaces and have something like "EXPORT_SYMBOL_TO(sym, modname)"?  It
> > doesn't work so well for exporting to a group of modules, but that seems
> > a reasonable line to draw anyway.
>
> That would explode quickly already even for my example "inet" namespace.
> It already has several modules.  I don't think so much duplication would be
> a good idea.

Yes, and if a symbol is already used by multiple modules, it's generically 
useful.  And if so, why restrict it to in-tree modules?

If your real intent is to bias against out-of-tree modules, let's just 
generate a list of in-tree module names, and restrict some or all exports to 
that set.

Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Friday 23 November 2007 01:25, Rusty Russell wrote:
> On Thursday 22 November 2007 22:05:45 Christoph Hellwig wrote:
> > On Thu, Nov 22, 2007 at 02:56:22PM +1100, Rusty Russell wrote:
> > >     This is an interesting idea, thanks for the code!  My only question
> > > is whether we can get most of this benefit by dropping the indirection
> > > of namespaces and have something like "EXPORT_SYMBOL_TO(sym, modname)"?
> > >  It doesn't work so well for exporting to a group of modules, but that
> > > seems a reasonable line to draw anyway.
> >
> > I'd say exporting to a group of modules is the main use case.  E.g. in
> > scsi there would be symbols exported to transport class modules only
> > or lots of the vfs_ symbols would be exported only to stackable
> > filesystems or nfsd.
>
> That's my point.  If there's a whole class of modules which can use a
> symbol, why are we ruling out external modules? 

The point is to get cleaner interfaces. Anything which is kind of internal
should only be used by closely related in tree modules which can be updated. 
Point of is not to be some kind of license enforcer or similar, there 
are already other mechanisms for that. Just to get the set of really
public kernel interfaces down to a manageable level.

But I still think exporting only to a single module would be to limiting 
for this case even. It would work for the TCP<->ipv6.ko post child,
but not for some of the other networking cases where it makes sense.

> If that's what you want, 
> why not have a list of permitted modules compiled into the kernel and allow
> no others?

That would not make the relationship explicit, which would not further
the goal.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Dave Young]

On Nov 23, 2007 2:19 AM, Andi Kleen <ak@suse.de> wrote:
>
> > Andy, I like your idea.  IMHO, as Rusty said a simple EXPORT_SYMBOL_TO
> > is better.
>
> I don't think so. e.g. tcpcong would be very very messy this way.
>
> > And I wonder if it is possible to export to something like  the struct
> > device_driver? If it's possible then it will not limited to modules.
>
> Not sure I follow you. Can you expand?
>
I know little about module internal, so if I'm wrong please just don't
mind, please  point out or just ignore.

Kernel symbols could apply to kernel object model, doesn't it?
I just think that because the device_driver have a mod_name member
(for built-in module), so if something can be done as device driver is
registered.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Friday 23 November 2007 12:36:22 Andi Kleen wrote:
> On Friday 23 November 2007 01:25, Rusty Russell wrote:
> > That's my point.  If there's a whole class of modules which can use a
> > symbol, why are we ruling out external modules?
>
> The point is to get cleaner interfaces.

But this doesn't change interfaces at all.  It makes modules fail to load 
unless they're on a permitted list, which now requires maintenance.

> Anything which is kind of internal 
> should only be used by closely related in tree modules which can be
> updated.

Is there evidence that this is a problem for us?  Are there any interfaces 
you've restricted so far which are causing problems?

> Point of is not to be some kind of license enforcer or similar, 
> there are already other mechanisms for that. Just to get the set of really
> public kernel interfaces down to a manageable level.

Why do we care what a "really public"?  We treat them all the same, as 
changeable interfaces.  ie.  None of them are "really public".

For example, you put all the udp functions in the "udp" namespace.  But what 
have we gained?  What has become easier to maintain?  All those function 
start with "udp_": are people having trouble telling what they're for?

If you really want to reduce "public interfaces" then it's much simpler to 
mark explicitly what out-of-tree modules can use.  We can have a list of 
symbol names in include/linux/public-exports.h.

I just don't see what problems this separation solves.
Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Fri, Nov 23, 2007 at 02:35:05PM +1100, Rusty Russell wrote:
> On Friday 23 November 2007 12:36:22 Andi Kleen wrote:
> > On Friday 23 November 2007 01:25, Rusty Russell wrote:
> > > That's my point.  If there's a whole class of modules which can use a
> > > symbol, why are we ruling out external modules?
> >
> > The point is to get cleaner interfaces.
> 
> But this doesn't change interfaces at all.  It makes modules fail to load 
> unless they're on a permitted list, which now requires maintenance.

The modules wouldn't be using the internal interfaces in the first
place with name spaces in place. This serves as a documentation
on what is considered internal. And if some obscure module (in or
out of tree) wants to use an internal interface they first have
to send the module maintainer a patch and get some review this way.

I believe that is fairly important in tree too because the 
kernel has become so big now that review cannot be the only
enforcement mechanism for this anymore.

Another secondary reason is that there are too many exported interfaces
in general. Several distributions have policies that require to 
keep the changes to these exported interfaces minimal and that
is very hard with thousands of exported symbol.  With name spaces
the number of truly publicly exported symbols will hopefully
shrink to a much smaller, more manageable set.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Saturday 24 November 2007 06:53:30 Andi Kleen wrote:
> This serves as a documentation 
> on what is considered internal. And if some obscure module (in or
> out of tree) wants to use an internal interface they first have
> to send the module maintainer a patch and get some review this way.

So, you're saying that there's a problem with in-tree modules using symbols 
they shouldn't?  Can you give an example?

> I believe that is fairly important in tree too because the
> kernel has become so big now that review cannot be the only
> enforcement mechanism for this anymore.

If people aren't reviewing, this won't make them review.  I don't think the 
problem is that people are conniving to avoid review.

> Another secondary reason is that there are too many exported interfaces
> in general.

Probably, but this doesn't reduce it.  

> Several distributions have policies that require to 
> keep the changes to these exported interfaces minimal and that
> is very hard with thousands of exported symbol.  With name spaces
> the number of truly publicly exported symbols will hopefully
> shrink to a much smaller, more manageable set.

*This* makes sense.  But it's not clear that the burden should be placed on 
kernel coders.  You can create a list yourself.  How do I tell the difference 
between "truly publicly exported" symbols and others?

If a symbol has more than one in-tree user, it's hard to argue against an 
out-of-tree module using the symbol, unless you're arguing against *all* 
out-of-tree modules.

Sorry,
Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Sat, Nov 24, 2007 at 03:53:34PM +1100, Rusty Russell wrote:
> So, you're saying that there's a problem with in-tree modules using symbols 
> they shouldn't?  Can you give an example?
> 
> > I believe that is fairly important in tree too because the
> > kernel has become so big now that review cannot be the only
> > enforcement mechanism for this anymore.
> 
> If people aren't reviewing, this won't make them review.  I don't think the 

With millions of LOC the primary maintainers cannot review everything.
It's not that anybody is doing a bad job -- it is just so much code
that explicit mechanisms are better than implicit contracts.

> problem is that people are conniving to avoid review.

No of course not -- it is just too much code to let everything
be reviewed by the core subsystem maintainers. But with explicit
marking of internal symbols they would need to look at it because
the relationship will be clearly spelled out in the code.

> > Several distributions have policies that require to 
> > keep the changes to these exported interfaces minimal and that
> > is very hard with thousands of exported symbol.  With name spaces
> > the number of truly publicly exported symbols will hopefully
> > shrink to a much smaller, more manageable set.
> 
> *This* makes sense.  But it's not clear that the burden should be placed on 
> kernel coders.  You can create a list yourself.  How do I tell the difference 
> between "truly publicly exported" symbols and others?

Out of tree solutions generally do not scale.  Nobody else can 
keep up with 2+ Million changes each merge window.

> 
> If a symbol has more than one in-tree user, it's hard to argue against an 

There are still classes of drivers. e.g. for the SCSI example: SD,SG,SR etc.
are more internal while low level drivers like aic7xxx are clearly external
drivers.

> out-of-tree module using the symbol, unless you're arguing against *all* 
> out-of-tree modules.

No, actually namespaces kind of help out of tree modules. Once they only
use interfaces that are really generic driver interfaces and fairly stable
their authors will have much less pain forward porting to newer kernel
version. But currently the authors cannot even know what is an instable
internal interface and what is a generic relatively stable driver level
interface. Namespaces are a mechanism to make this all explicit.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Roland Dreier]

 > This patch allows to export symbols only for specific modules by 
 > introducing symbol name spaces. A module name space has a white
 > list of modules that are allowed to import symbols for it; all others
 > can't use the symbols.
 > 
 > It adds two new macros: 
 > 
 > MODULE_NAMESPACE_ALLOW(namespace, module);

I definitely like the idea of organizing exported symbols into
namespaces.  However, I feel like it would make more sense to have
something like

MODULE_NAMESPACE_IMPORT(namespace);

inside the modules that use a namespace.  This matches the way C
preprocessor #includes, C++ namespaces, perl "use", etc. works and so
it is probably closer to how programmers think.  It does weaken the
enforcement of review a little bit, since there are no changes to the
site where things are exported to catch, but git makes it easy to
sneak that kind of change in anyway.

The practical benefits are that you don't end up with stupid patch
conflicts caused by one patch adding MODULE_NAMESPACE_ALLOW() for
module "foo" and another patch adding it for module "bar" at the same
place, and that it becomes simpler for people to test or deliver, say,
a new TCP congestion module without having to rebuild the whole kernel
(which is an especially huge pain for distro kernels).

In any case I would make use of this in whatever form it gets merged
in -- Mellanox ConnectX hardware can operate simultaneously as an
InfiniBand adapter and a 10Gb ethernet NIC, and so my driver is split
up into a low-level mlx4_core driver that exports symbols for other
mlx4 drivers to use; clearly it only makes sense to export them to
other parts of the driver, and in this case the difference between
"ALLOW" and "IMPORT" semantics is not a big deal.

 - R.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Roland Dreier]

 > Yes, and if a symbol is already used by multiple modules, it's generically 
 > useful.  And if so, why restrict it to in-tree modules?

I agree that we shouldn't make things too hard for out-of-tree
modules, but I disagree with your first statement: there clearly is a
large class of symbols that are used by multiple modules but which are
not generically useful -- they are only useful by a certain small class
of modules.

 - R.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Saturday 24 November 2007 23:39:43 Andi Kleen wrote:
> On Sat, Nov 24, 2007 at 03:53:34PM +1100, Rusty Russell wrote:
> > So, you're saying that there's a problem with in-tree modules using
> > symbols they shouldn't?  Can you give an example?

[ Note: no response to this ]

> > If people aren't reviewing, this won't make them review.  I don't think
> > the
>
> With millions of LOC the primary maintainers cannot review everything.
> It's not that anybody is doing a bad job -- it is just so much code
> that explicit mechanisms are better than implicit contracts.
>
> > problem is that people are conniving to avoid review.
>
> No of course not -- it is just too much code to let everything
> be reviewed by the core subsystem maintainers. But with explicit
> marking of internal symbols they would need to look at it because
> the relationship will be clearly spelled out in the code.

No, a one-line patch adding the module to the set is all they'd see.  There's 
no reason to think this will cause more review.

> > > Several distributions have policies that require to
> > > keep the changes to these exported interfaces minimal and that
> > > is very hard with thousands of exported symbol.  With name spaces
> > > the number of truly publicly exported symbols will hopefully
> > > shrink to a much smaller, more manageable set.
> >
> > *This* makes sense.  But it's not clear that the burden should be placed
> > on kernel coders.  You can create a list yourself.  How do I tell the
> > difference between "truly publicly exported" symbols and others?
>
> Out of tree solutions generally do not scale.  Nobody else can
> keep up with 2+ Million changes each merge window.
>
> > If a symbol has more than one in-tree user, it's hard to argue against an
>
> There are still classes of drivers. e.g. for the SCSI example: SD,SG,SR
> etc. are more internal while low level drivers like aic7xxx are clearly
> external drivers.

Then mark those symbols internal and only allow concurrently-built modules to 
access them.  That's simpler and requires much less maintenance than your 
solution.

> > out-of-tree module using the symbol, unless you're arguing against *all*
> > out-of-tree modules.
>
> No, actually namespaces kind of help out of tree modules. Once they only
> use interfaces that are really generic driver interfaces and fairly stable
> their authors will have much less pain forward porting to newer kernel
> version. But currently the authors cannot even know what is an instable
> internal interface and what is a generic relatively stable driver level
> interface. Namespaces are a mechanism to make this all explicit.

So in your head you have a notion of a kernel API, and you're trying to make 
that API explicit in the code.

Sorry, but no.
Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Monday 26 November 2007 07:29:39 Roland Dreier wrote:
>  > Yes, and if a symbol is already used by multiple modules, it's
>  > generically useful.  And if so, why restrict it to in-tree modules?
>
> I agree that we shouldn't make things too hard for out-of-tree
> modules, but I disagree with your first statement: there clearly is a
> large class of symbols that are used by multiple modules but which are
> not generically useful -- they are only useful by a certain small class
> of modules.

If it is so clear, you should be able to easily provide examples?

Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Monday 26 November 2007 07:27:03 Roland Dreier wrote:
>  > This patch allows to export symbols only for specific modules by
>  > introducing symbol name spaces. A module name space has a white
>  > list of modules that are allowed to import symbols for it; all others
>  > can't use the symbols.
>  >
>  > It adds two new macros:
>  >
>  > MODULE_NAMESPACE_ALLOW(namespace, module);
>
> I definitely like the idea of organizing exported symbols into
> namespaces.  However, I feel like it would make more sense to have
> something like
>
> MODULE_NAMESPACE_IMPORT(namespace);

Except C doesn't have namespaces and this mechanism doesn't create them.  So 
this is just complete and utter makework; as I said before, noone's going to 
confuse all those udp_* functions if they're not in the udp namespace.

For better or worse, this is not C++.

Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Roland Dreier]

 > > I agree that we shouldn't make things too hard for out-of-tree
 > > modules, but I disagree with your first statement: there clearly is a
 > > large class of symbols that are used by multiple modules but which are
 > > not generically useful -- they are only useful by a certain small class
 > > of modules.
 > 
 > If it is so clear, you should be able to easily provide examples?

Sure -- Andi's example of symbols required only by TCP congestion
modules; the SCSI internals that Christoph wants to mark; the symbols
exported by my mlx4_core driver (which I admit are currently only used
by the mlx4_ib driver, but which will also be used by at least the
ethernet NIC driver for the same hardware).  I thought this was
already covered repeatedly in the thread and indeed in Andi's code so
there was no need to repeat it...

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Roland Dreier]

 > Except C doesn't have namespaces and this mechanism doesn't create them.  So 
 > this is just complete and utter makework; as I said before, noone's going to 
 > confuse all those udp_* functions if they're not in the udp namespace.

I don't understand why you're so opposed to organizing the kernel's
exported symbols in a more self-documenting way.  It seems pretty
clear to me that having a mechanism that requires modules to make
explicit which (semi-)internal APIs makes reviewing easier, makes it
easier to communicate "please don't use that API" to module authors,
and takes at least a small step towards bringing the kernel's exported
API under control.  What's the real downside?

 - R.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Stephen Hemminger]

On Mon, 26 Nov 2007 12:28:14 +1100
Rusty Russell <rusty@rustcorp.com.au> wrote:

> On Monday 26 November 2007 07:27:03 Roland Dreier wrote:
> >  > This patch allows to export symbols only for specific modules by
> >  > introducing symbol name spaces. A module name space has a white
> >  > list of modules that are allowed to import symbols for it; all others
> >  > can't use the symbols.
> >  >
> >  > It adds two new macros:
> >  >
> >  > MODULE_NAMESPACE_ALLOW(namespace, module);
> >
> > I definitely like the idea of organizing exported symbols into
> > namespaces.  However, I feel like it would make more sense to have
> > something like
> >
> > MODULE_NAMESPACE_IMPORT(namespace);
> 
> Except C doesn't have namespaces and this mechanism doesn't create them.  So 
> this is just complete and utter makework; as I said before, noone's going to 
> confuse all those udp_* functions if they're not in the udp namespace.
> 
> For better or worse, this is not C++.
> 


Agreed. On first glance, I was intrigued but:

1) Why is everyone so concerned that export symbol space is large?
	- does it cost cpu or running memory?
	- does it cause bugs?
	- or are you just worried about "evil modules"?

2) These aren't real namespaces
	- all global names still have to be unique
	- still have to handle the "non-modular build" namespace conflicts
	- there isn't a big problem with conflicting symbols today.

So why bother adding complexity.
-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Roland Dreier]

 > Agreed. On first glance, I was intrigued but:
 > 
 > 1) Why is everyone so concerned that export symbol space is large?
 > 	- does it cost cpu or running memory?
 > 	- does it cause bugs?
 > 	- or are you just worried about "evil modules"?
 > 
 > 2) These aren't real namespaces
 > 	- all global names still have to be unique
 > 	- still have to handle the "non-modular build" namespace conflicts
 > 	- there isn't a big problem with conflicting symbols today.

Perhaps changing the name from "namespace" to "interface" would help?
Then a module could have something like

MODULE_USE_INTERFACE(foo);

and I think that makes it clearer what the advantage of this is: it
marks symbols as being part of a certain interface, requires modules
that use that interface to declare that use explicitly, and allows
reviewers to say "Hey why is this code using the scsi interface when
it's a webcam driver?"

 - R.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Monday 26 November 2007 16:58:08 Roland Dreier wrote:
>  > > I agree that we shouldn't make things too hard for out-of-tree
>  > > modules, but I disagree with your first statement: there clearly is a
>  > > large class of symbols that are used by multiple modules but which are
>  > > not generically useful -- they are only useful by a certain small
>  > > class of modules.
>  >
>  > If it is so clear, you should be able to easily provide examples?
>
> Sure -- Andi's example of symbols required only by TCP congestion
> modules;

Exactly.  Why exactly should someone not write a new TCP congestion module?

> the SCSI internals that Christoph wants to mark

He didn't justify those though, either.

> ; the symbols  exported by my mlx4_core driver (which I admit are
> currently only used 
> by the mlx4_ib driver, but which will also be used by at least the
> ethernet NIC driver for the same hardware).

Right.  So presumably there will only ever be two drivers using this core 
code, so no new users will ever be written?  Now we've found one use case, is 
it worth the complexity of namespaces?  Is it worth the halfway point of 
export-to-module?

What problem will it solve?

> I thought this was 
> already covered repeatedly in the thread and indeed in Andi's code so
> there was no need to repeat it...

No, we've seen the solution and various people applying it.  I'm still trying 
to discover the problem it's solving.

Hope that helps,
Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Monday 26 November 2007 17:15:44 Roland Dreier wrote:
>  > Except C doesn't have namespaces and this mechanism doesn't create them.
>  >  So this is just complete and utter makework; as I said before, noone's
>  > going to confuse all those udp_* functions if they're not in the udp
>  > namespace.
>
> I don't understand why you're so opposed to organizing the kernel's
> exported symbols in a more self-documenting way.

No, I was the one who moved exports near their declarations.  That's 
organised.  I just don't see how this new "organization" will help: oh good, 
I won't accidentally use the udp functions any more?!?

> It seems pretty   
> clear to me that having a mechanism that requires modules to make
> explicit which (semi-)internal APIs makes reviewing easier

Perhaps you've got lots of patches were people are using internal APIs they 
shouldn't?

> , makes it 
> easier to communicate "please don't use that API" to module authors,

Well, introduce an EXPORT_SYMBOL_INTERNAL().  It's a lot less code.  But you'd 
still need to show that people are having trouble knowing what APIs to use.

> and takes at least a small step towards bringing the kernel's exported
> API under control.

There is no "exported API" to bring under control.  There are symbols we 
expose for the kernel's own use which can be used by external modules at 
their own risk.  

> What's the real downside? 

No.  That's the wrong question.  What's the real upside?

Let's not put code in the core because "it doesn't seem to hurt".

I'm sure you think there's a real problem, but I'm still waiting for someone 
to *show* it to me.  Then we can look at solutions.

Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Tom Tucker]

On Tue, 2007-11-27 at 15:49 +1100, Rusty Russell wrote:
> On Monday 26 November 2007 17:15:44 Roland Dreier wrote:
> >  > Except C doesn't have namespaces and this mechanism doesn't create them.
> >  >  So this is just complete and utter makework; as I said before, noone's
> >  > going to confuse all those udp_* functions if they're not in the udp
> >  > namespace.
> >
> > I don't understand why you're so opposed to organizing the kernel's
> > exported symbols in a more self-documenting way.
> 
> No, I was the one who moved exports near their declarations.  That's 
> organised.  I just don't see how this new "organization" will help: oh good, 
> I won't accidentally use the udp functions any more?!?
> 
> > It seems pretty   
> > clear to me that having a mechanism that requires modules to make
> > explicit which (semi-)internal APIs makes reviewing easier
> 
> Perhaps you've got lots of patches were people are using internal APIs they 
> shouldn't?
> 

Maybe the issue is "who can tell" since what is external and what is
internal is not explicitly defined?

> > , makes it 
> > easier to communicate "please don't use that API" to module authors,
> 
> Well, introduce an EXPORT_SYMBOL_INTERNAL().  It's a lot less code.  But you'd 
> still need to show that people are having trouble knowing what APIs to use.

> > and takes at least a small step towards bringing the kernel's exported
> > API under control.
> 
> There is no "exported API" to bring under control.  

Hmm...apparently, there are those that are struggling...

> There are symbols we 
> expose for the kernel's own use which can be used by external modules at 
> their own risk.  
> 
> > What's the real downside? 
> 
> No.  That's the wrong question.  What's the real upside?

Explicitly documenting what comprises the kernel API (external,
supported) and what comprises the kernel implementation (internal, not
supported).

> 
> Let's not put code in the core because "it doesn't seem to hurt".
> 

agreed.

> I'm sure you think there's a real problem, but I'm still waiting for someone 
> to *show* it to me.  Then we can look at solutions.

I think the benefits should include:

- forcing developers to identify their exports as part of the
implementation or as part of the kernel API

- making it easier for reviewers to identify when developers are adding
to the kernel API and thereby focusing the appropriate level of review
to the new function

- making it obvious to developers when they are binding their
implementation to a particular kernel release



> Rusty.
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

> > Perhaps you've got lots of patches were people are using internal APIs they 
> > shouldn't?
> > 
> 
> Maybe the issue is "who can tell" since what is external and what is
> internal is not explicitly defined?

Exactly.  Or rather it is not defined on the module level. We got 
"static" of course, but I think we should have a similar mechanism
on a module level.


> Explicitly documenting what comprises the kernel API (external,
> supported) 

It would not be fully supported either -- can still change etc. --
but there is a reasonable expectation that those external
APIs will change less often than internal interfaces.

> - forcing developers to identify their exports as part of the
> implementation or as part of the kernel API

That is EXPORT_SYMBOL already. The trouble is just that it covers
too much. My patchkit is trying to limit it again for a specific
use case -- exporting an "internal" interface to another module.
Or rather a set of modules. 

Standard example is TCP: TCP exports nearly everything and the
single user is the TCP code in ipv6.ko. Instead those symbols should
be limited to be only accessable to ipv6.ko. 

The reason I went with the more generic namespace mechanism
instead of EXPORT_SYMBOL_TO() is that ipv6 is ever split up
it would still work. 

Also using namespaces doesn't have any more overhead than
EXPORT_SYMBOL_TO() and the complexity is about the same
(not very much anyways -- just look at the patches) 

> - making it easier for reviewers to identify when developers are adding
> to the kernel API and thereby focusing the appropriate level of review
> to the new function

That is another reason.
 
-ANdi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Tue, Nov 27, 2007 at 03:26:52PM +1100, Rusty Russell wrote:
> On Monday 26 November 2007 16:58:08 Roland Dreier wrote:
> >  > > I agree that we shouldn't make things too hard for out-of-tree
> >  > > modules, but I disagree with your first statement: there clearly is a
> >  > > large class of symbols that are used by multiple modules but which are
> >  > > not generically useful -- they are only useful by a certain small
> >  > > class of modules.
> >  >
> >  > If it is so clear, you should be able to easily provide examples?
> >
> > Sure -- Andi's example of symbols required only by TCP congestion
> > modules;
> 
> Exactly.  Why exactly should someone not write a new TCP congestion module?

Agreed the congestion modules are a corner case. I even mentioned that in the
patch. I would be happy to drop that one if that is the consensus.
It was more done as a example anyways. That is why I made it an separate
namespace from "tcp"

But for many other TCP symbols it makes a lot of sense: all the functions
only used by tcp_ipv6.c. If someone wants to write support for a "IPv7" or
similar they really should do it in tree. So I think the "tcp" and  "inet"
namespaces make a lot of sense.

> Right.  So presumably there will only ever be two drivers using this core 
> code, so no new users will ever be written?  Now we've found one use case, is 

If there are new users they will need to get proper review and should
be in tree.  MODULE_ALLOW() enforces that.

> it worth the complexity of namespaces?  Is it worth the halfway point of 

What complexity? You're always claiming it is complex. It isn't really.

> export-to-module?
> 

> No, we've seen the solution and various people applying it.  I'm still trying 
> to discover the problem it's solving.

Again for rusty @)

Goals are:
- Limit the interfaces available for out of tree modules to reasonably 
stable ones that are already used by a larger set of drivers.

This can also have further downstream advantages.
For example it might be a reasonable future rule to require all unconditionally
EXPORT_SYMBOL()s to have a complete LinuxDoc documentation entry.

- Explicitely declare in source what is clearly internal and not intended to
be a generally usable interface.

e.g. for the LinuxDoc example above such internal functions don't necessarily
need full LinuxDoc documentation.

- Force review from core maintainers for use of such internal interfaces

- Limit size of exported API to make stable ABIs for enterprise
distributions easier
[Yes I know that is not a popular topic on l-k, but it's a day-to-day
problem for these distros and out of tree solutions do not work]

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Herbert Xu]

Andi Kleen <andi@firstfloor.org> wrote:
> On Tue, Nov 27, 2007 at 03:26:52PM +1100, Rusty Russell wrote:
>> On Monday 26 November 2007 16:58:08 Roland Dreier wrote:
>> >  > > I agree that we shouldn't make things too hard for out-of-tree
>> >  > > modules, but I disagree with your first statement: there clearly is a
>> >  > > large class of symbols that are used by multiple modules but which are
>> >  > > not generically useful -- they are only useful by a certain small
>> >  > > class of modules.
>> >  >
>> >  > If it is so clear, you should be able to easily provide examples?
>> >
>> > Sure -- Andi's example of symbols required only by TCP congestion
>> > modules;
>> 
>> Exactly.  Why exactly should someone not write a new TCP congestion module?
> 
> Agreed the congestion modules are a corner case. I even mentioned that in the
> patch. I would be happy to drop that one if that is the consensus.
> It was more done as a example anyways. That is why I made it an separate
> namespace from "tcp"
> 
> But for many other TCP symbols it makes a lot of sense: all the functions
> only used by tcp_ipv6.c. If someone wants to write support for a "IPv7" or
> similar they really should do it in tree. So I think the "tcp" and  "inet"
> namespaces make a lot of sense.

OK, short of making IPv4 a module (which would be a worthy task :)
do you have an example where a symbol is used by more than one module
but needs to be put into a namespace?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

> OK, short of making IPv4 a module (which would be a worthy task :)

At some point there were patches, it is probably not very difficult.
But DaveM resisted at some point because he didn't want people
to replace the network stack (although I personally don't have a problem
with that)

> do you have an example where a symbol is used by more than one module
> but needs to be put into a namespace?

For SCSI: SD,SR,SG etc.
For Networking: e.g. symbols i put into inet, which are only
used by protocols (sctp, dccp, udplite, ipv6)
I already caught someone doing something wrong with that BTW -- 
wanrouter clearly does some things it shouldn't be doing. 
Or the fib namespace, where all the fib functions should be only
used by the two fib_* modules and ipv6/decnet.
For KVM: the exports used by kvm_amd/intel
For arch/x86: e.g. e820_*, IO_APIC_get_PCI_irq_vector, swiotlb, 
a lot of stuff only used by acpi processor.c, pcibios*, etc.etc.
Roland gave another example for Infiniband.

Also in general it allows flexibility later if modules get split.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Herbert Xu]

On Tue, Nov 27, 2007 at 03:12:42PM +0100, Andi Kleen wrote:
>
> For Networking: e.g. symbols i put into inet, which are only
> used by protocols (sctp, dccp, udplite, ipv6)

Wait, that's exactly Rusty's point (I think :)

These symbols are exported because they're needed by protocols.
If they weren't available to everyone then it would be difficult
to start writing new protocols.

> I already caught someone doing something wrong with that BTW -- 
> wanrouter clearly does some things it shouldn't be doing. 

Can you be more precise?

> Or the fib namespace, where all the fib functions should be only
> used by the two fib_* modules and ipv6/decnet.

Again, if it's used by decnet then it sounds like it should be
exported because new protocol families may need them.

So based on the network code at least I'm kind of starting to
agree with Rusty now: if a symbol is needed by more than one
in-tree module chances are we want it to be exported for all.

Although I admit I haven't examined your examples elsewhere.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Jonathan Corbet]

Rusty said:

> Well, introduce an EXPORT_SYMBOL_INTERNAL().  It's a lot less code.  But you'd 
> still need to show that people are having trouble knowing what APIs to use.

Might the recent discussion on the exporting of sys_open() and
sys_read() be an example here?  There would appear to be a consensus
that people should not have used those functions, but they are now
proving difficult to unexport.

Perhaps the best use of the namespace stuff might be for *future*
exports which are needed to help make the mainline kernel fully modular,
but which are not meant to be part of a more widely-used API?

jon

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Tue, Nov 27, 2007 at 08:43:24AM -0700, Jonathan Corbet wrote:
> Rusty said:
> 
> > Well, introduce an EXPORT_SYMBOL_INTERNAL().  It's a lot less code.  But you'd 
> > still need to show that people are having trouble knowing what APIs to use.
> 
> Might the recent discussion on the exporting of sys_open() and
> sys_read() be an example here?  There would appear to be a consensus
> that people should not have used those functions, but they are now
> proving difficult to unexport.

That is a good example yes.

> Perhaps the best use of the namespace stuff might be for *future*
> exports which are needed to help make the mainline kernel fully modular,
> but which are not meant to be part of a more widely-used API?

Not sure about future only, but yes that is its intention.
Thanks for putting it clearly.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Christoph Hellwig]

On Tue, Nov 27, 2007 at 08:43:24AM -0700, Jonathan Corbet wrote:
> Might the recent discussion on the exporting of sys_open() and
> sys_read() be an example here?  There would appear to be a consensus
> that people should not have used those functions, but they are now
> proving difficult to unexport.

They're not.  The exports aren't used anymore and could just go away
any time.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Adrian Bunk]

On Mon, Nov 26, 2007 at 11:35:42PM -0600, Tom Tucker wrote:
> On Tue, 2007-11-27 at 15:49 +1100, Rusty Russell wrote:
>...
> > No.  That's the wrong question.  What's the real upside?
> 
> Explicitly documenting what comprises the kernel API (external,
> supported) and what comprises the kernel implementation (internal, not
> supported).
>...

There is not, never was, and never will be, any supported external API 
of the kernel.
 
cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Adrian Bunk]

On Tue, Nov 27, 2007 at 10:02:22AM +0100, Andi Kleen wrote:
>...
> That is EXPORT_SYMBOL already. The trouble is just that it covers
> too much. My patchkit is trying to limit it again for a specific
> use case -- exporting an "internal" interface to another module.
> Or rather a set of modules. 
> 
> Standard example is TCP: TCP exports nearly everything and the
> single user is the TCP code in ipv6.ko. Instead those symbols should
> be limited to be only accessable to ipv6.ko. 
>...

Let's forget about external modules that are anyway irrelevant for 
upstream kernel development.

Do you have past examples where this would have brought advantages 
for the upstream kernel justifying all the work required for creating 
and maintaining these namespaces?

IOW, where modules were submitted for upstream inclusion and merging 
them was impossible or much harder only because they were developed 
aginst the wrong API?

> -ANdi

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Tom Tucker]

On Tue, 2007-11-27 at 18:15 +0100, Adrian Bunk wrote:
> On Mon, Nov 26, 2007 at 11:35:42PM -0600, Tom Tucker wrote:
> > On Tue, 2007-11-27 at 15:49 +1100, Rusty Russell wrote:
> >...
> > > No.  That's the wrong question.  What's the real upside?
> > 
> > Explicitly documenting what comprises the kernel API (external,
> > supported) and what comprises the kernel implementation (internal, not
> > supported).
> >...
> 
> There is not, never was, and never will be, any supported external API 
> of the kernel.

Philosophically I understand what you're saying, but in practical terms
there is the issue of managing core API like kmalloc. Although kmalloc
_could_ change, doing so would be extremely painful. In fact anyone who
proposed such a change would have to have a profoundly powerful argument
as to why it was necessary.

I think this patchset is an attempt to make it easier to identify and
review these kinds of interfaces.

>  
> cu
> Adrian
> 

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Adrian Bunk]

On Tue, Nov 27, 2007 at 11:45:37AM -0600, Tom Tucker wrote:
> 
> On Tue, 2007-11-27 at 18:15 +0100, Adrian Bunk wrote:
> > On Mon, Nov 26, 2007 at 11:35:42PM -0600, Tom Tucker wrote:
> > > On Tue, 2007-11-27 at 15:49 +1100, Rusty Russell wrote:
> > >...
> > > > No.  That's the wrong question.  What's the real upside?
> > > 
> > > Explicitly documenting what comprises the kernel API (external,
> > > supported) and what comprises the kernel implementation (internal, not
> > > supported).
> > >...
> > 
> > There is not, never was, and never will be, any supported external API 
> > of the kernel.
> 
> Philosophically I understand what you're saying, but in practical terms
> there is the issue of managing core API like kmalloc. Although kmalloc
> _could_ change, doing so would be extremely painful. In fact anyone who
> proposed such a change would have to have a profoundly powerful argument
> as to why it was necessary.

The latter should at least in theory be required for all changes no 
matter how core they are...

> I think this patchset is an attempt to make it easier to identify and
> review these kinds of interfaces.

As long as the submitter fixes all in-kernel users these interfaces are 
not handled differently from interfaces with fewer users.

And I remember at least one commit that changed > 1000 files because it 
changed a frequently used driver API. [1]

cu
Adrian

[1] commit 7d12e780e003f93433d49ce78cfedf4b4c52adc5

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Dave Jones]

On Mon, Nov 26, 2007 at 10:25:33AM -0800, Stephen Hemminger wrote:
 
 > 1) Why is everyone so concerned that export symbol space is large?
 > 	- does it cost cpu or running memory?
 > 	- does it cause bugs?
 > 	- or are you just worried about "evil modules"?

To clarify something here, by "evil", don't necessarily think "binary only". 

Out of tree modules are frequently using symbols that they shouldn't be.
Because they get no peer-review here, they 'get away with it' for the most part.
Until distro vendors push rebased kernel updates that removed exports that
should never have been exported, and suddenly people like me get bombed
with "Fedora broke my xyz driver" mails.

Reducing the opportunity for people to screw things up is a good thing.
If a symbol is exported, most out-of-tree driver authors seem to
think its "fair game" to use it.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

On Tue, 27 Nov 2007 15:12:42 +0100, Andi Kleen said:
> > OK, short of making IPv4 a module (which would be a worthy task :)
> 
> At some point there were patches, it is probably not very difficult.
> But DaveM resisted at some point because he didn't want people
> to replace the network stack (although I personally don't have a problem
> with that)

Personally, I wouldn't find replacing the ipv4 stack very interesting.

However, stress-testing your system for IPv6-readiness by doing 'rmmod ipv4' :)

(Though I admit it's something I'd *try* if it was available, but certainly
not sufficient for a reason to do it...)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Adrian Bunk]

On Tue, Nov 27, 2007 at 02:00:37PM -0500, Dave Jones wrote:
> On Mon, Nov 26, 2007 at 10:25:33AM -0800, Stephen Hemminger wrote:
>  
>  > 1) Why is everyone so concerned that export symbol space is large?
>  > 	- does it cost cpu or running memory?
>  > 	- does it cause bugs?
>  > 	- or are you just worried about "evil modules"?
> 
> To clarify something here, by "evil", don't necessarily think "binary only". 
> 
> Out of tree modules are frequently using symbols that they shouldn't be.
> Because they get no peer-review here, they 'get away with it' for the most part.
> Until distro vendors push rebased kernel updates that removed exports that
> should never have been exported, and suddenly people like me get bombed
> with "Fedora broke my xyz driver" mails.
>...

The real problem is that these drivers are not in the upstream kernel.

Are there common reasons why these drivers are not upstream?

> 	Dave

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rick Jones]

> The real problem is that these drivers are not in the upstream kernel.
> 
> Are there common reasons why these drivers are not upstream?

One might be that upstream has not accepted them.  Anything doing or 
smelling of TOE comes to mind right away.

rick jones

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Dave Jones]

On Tue, Nov 27, 2007 at 10:09:42PM +0100, Adrian Bunk wrote:
 > On Tue, Nov 27, 2007 at 02:00:37PM -0500, Dave Jones wrote:
 > > On Mon, Nov 26, 2007 at 10:25:33AM -0800, Stephen Hemminger wrote:
 > >  
 > >  > 1) Why is everyone so concerned that export symbol space is large?
 > >  > 	- does it cost cpu or running memory?
 > >  > 	- does it cause bugs?
 > >  > 	- or are you just worried about "evil modules"?
 > > 
 > > To clarify something here, by "evil", don't necessarily think "binary only". 
 > > 
 > > Out of tree modules are frequently using symbols that they shouldn't be.
 > > Because they get no peer-review here, they 'get away with it' for the most part.
 > > Until distro vendors push rebased kernel updates that removed exports that
 > > should never have been exported, and suddenly people like me get bombed
 > > with "Fedora broke my xyz driver" mails.
 > >...
 > 
 > The real problem is that these drivers are not in the upstream kernel.

You're preaching to the choir.

 > Are there common reasons why these drivers are not upstream?

It varies case by case.

	Dave

-- 
http://www.codemonkey.org.uk

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Adrian Bunk]

On Tue, Nov 27, 2007 at 01:15:23PM -0800, Rick Jones wrote:
>> The real problem is that these drivers are not in the upstream kernel.
>>
>> Are there common reasons why these drivers are not upstream?
>
> One might be that upstream has not accepted them.  Anything doing or 
> smelling of TOE comes to mind right away.

Which modules doing or smelling of TOE do work with unmodified vendor 
kernels?

AFAIR TOE was both not a module and required some hooks in the network 
stack, so it's completely outside the scope of this thread.

> rick jones

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Jon Masters]

On Tue, 2007-11-27 at 15:49 +1100, Rusty Russell wrote:
> On Monday 26 November 2007 17:15:44 Roland Dreier wrote:

> > It seems pretty   
> > clear to me that having a mechanism that requires modules to make
> > explicit which (semi-)internal APIs makes reviewing easier
> 
> Perhaps you've got lots of patches were people are using internal APIs they 
> shouldn't?

With my "Enterprise" hat on, I can see where Andi was coming from
originally. Just like we do in RHEL, SuSE have a concept of a kernel ABI
and we too have a concept of a whitelist of symbols - a subset of kernel
ABI that is approved for use by third parties (this is nothing to do
with licensing, this is to do with ABI stability we try to ensure).

As part of figuring out what should and should not be used by external
modules (outside of the core kernel), we've gained a bit of experience,
and it would be nice to be able to help others - this is nothing to do
with upstream "ABI stability", but just to be of a public service by
documenting those functions that are never intended for modules but
which necessarily are exported because they have one or two users.

> > , makes it 
> > easier to communicate "please don't use that API" to module authors,
> 
> Well, introduce an EXPORT_SYMBOL_INTERNAL().  It's a lot less code.  But you'd 
> still need to show that people are having trouble knowing what APIs to use.

I suggested this exact idea privately at OLS. I still think it's the
best compromise, though I like bits of the namespace idea. An
EXPORT_SYMBOL_INTERNAL would indeed be trivial.

Jon.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

> With my "Enterprise" hat on, I can see where Andi was coming from
> originally. 

For the record my original motivation was to fix the "TCP exports everything
for ipv6.ko" case cleanly. I later realized that it would be useful for the
ABI stability issues too, but it was really not my primary motivation.
This is why I didn't even mention that in the original patch description.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Stephen Hemminger]

On Tue, 27 Nov 2007 23:37:43 +0100
Andi Kleen <andi@firstfloor.org> wrote:

> > With my "Enterprise" hat on, I can see where Andi was coming from
> > originally. 
> 
> For the record my original motivation was to fix the "TCP exports everything
> for ipv6.ko" case cleanly. I later realized that it would be useful for the
> ABI stability issues too, but it was really not my primary motivation.
> This is why I didn't even mention that in the original patch description.

Since ipv6 can never be removed because it references itself, the whole concept
of a modular ipv6 is flawed. Sometime ago a patch was sent to remove the ipv6 module
unload hooks but as I recall Dave rejected it believing the unload code could be
fixed.
-- 
Stephen Hemminger <shemminger@linux-foundation.org>

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

On Tue, 27 Nov 2007 22:09:42 +0100, Adrian Bunk said:

> Are there common reasons why these drivers are not upstream?

Well, on my laptop, I'm currently dragging along 3 out-of-tree kernel modules.
2 are well-known binary blobs so it's between me and the vendor, as usual.

The third is a USB webcam driver that happened to get caught at the wrong
end of the colorspace-conversion-in-kernel bunfight in the V4L playpen.
Somebody wants to figure out how to get the gspca drivers into the kernel,
they're at http://mxhaard.free.fr/download.html waiting for attention. ;)

(Don't look at *me*, I don't understand the code, or the bunfight - I just
happen to have one of the 244 supported webcams, and it works with that driver)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Tue, Nov 27, 2007 at 03:00:22PM -0800, Stephen Hemminger wrote:
> On Tue, 27 Nov 2007 23:37:43 +0100
> Andi Kleen <andi@firstfloor.org> wrote:
> 
> > > With my "Enterprise" hat on, I can see where Andi was coming from
> > > originally. 
> > 
> > For the record my original motivation was to fix the "TCP exports everything
> > for ipv6.ko" case cleanly. I later realized that it would be useful for the
> > ABI stability issues too, but it was really not my primary motivation.
> > This is why I didn't even mention that in the original patch description.
> 
> Since ipv6 can never be removed because it references itself, the whole concept

AFAIK that is obsolete anyways. It was done because it was feared it would
be broken, but at least the broken cases I knew about were all fixed
a long time ago. Most likely it could be safely removed.

> of a modular ipv6 is flawed. 

Modules that cannot be unloaded are still useful. Standard case: Distributions
like to offer an option to not use ipv6 because that is popular workaround
for the common "DNS server eats AAAA queries and causes delays" issue.
Forcing the user to rebuild the kernel for this wouldn't be practical.
If ipv6 wasn't modular that would be hard to do.

-Andi

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Tuesday 27 November 2007 16:35:42 Tom Tucker wrote:
> On Tue, 2007-11-27 at 15:49 +1100, Rusty Russell wrote:
> Explicitly documenting what comprises the kernel API (external,
> supported) and what comprises the kernel implementation (internal, not
> supported).

But the former is currently an empty set.

> - making it obvious to developers when they are binding their
> implementation to a particular kernel release

See, there's your problem.  All interfaces can, and will, change.  You're 
always binding yourself to a particular release.

So you're not proposing we mark what's not stable, you're arguing that we 
create a subset which is stable.

That's an argument we're not (yet) having.

Cheers,
Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Tuesday 27 November 2007 21:50:16 Andi Kleen wrote:
> Goals are:
> - Limit the interfaces available for out of tree modules to reasonably
> stable ones that are already used by a larger set of drivers.

Not the goals.  I haven't seen the *problem* yet.

> - Limit size of exported API to make stable ABIs for enterprise
> distributions easier
> [Yes I know that is not a popular topic on l-k, but it's a day-to-day
> problem for these distros and out of tree solutions do not work]

That's a real problem, and I sympathise with the idea of marking symbols as 
externally useful (or, practically, mark internal).

But we now need to decide what's "externally useful".  The currently line for 
exports is simple: someone in-tree needs it.  You dislike the suggestion to 
extend this to "if more than one in-tree needs it it's open".

Currently your criterion seems to be "does the maintainer hate external 
modules?" which I don't think will be what you want...

Cheers,
Rusty.

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rick Jones]

Adrian Bunk wrote:
> On Tue, Nov 27, 2007 at 01:15:23PM -0800, Rick Jones wrote:
> 
>>>The real problem is that these drivers are not in the upstream kernel.
>>>
>>>Are there common reasons why these drivers are not upstream?
>>
>>One might be that upstream has not accepted them.  Anything doing or 
>>smelling of TOE comes to mind right away.
> 
> 
> Which modules doing or smelling of TOE do work with unmodified vendor 
> kernels?

At the very real risk of further demonstrating my Linux vocabulary 
limitations, I believe there is a "Linux Sockets Acceleration" 
module/whatnot for NetXen and related 10G NICs, and a cxgb3_toe (?) 
module for Chelsio 10G NICs.

rick jones

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Tom Tucker]

On 11/27/07 7:27 PM, "Rusty Russell" <rusty@rustcorp.com.au> wrote:

> On Tuesday 27 November 2007 16:35:42 Tom Tucker wrote:
>> On Tue, 2007-11-27 at 15:49 +1100, Rusty Russell wrote:
>> Explicitly documenting what comprises the kernel API (external,
>> supported) and what comprises the kernel implementation (internal, not
>> supported).
> 
> But the former is currently an empty set.
> 

Yes, I overstated this.

>> - making it obvious to developers when they are binding their
>> implementation to a particular kernel release
> 
> See, there's your problem.  All interfaces can, and will, change.  You're
> always binding yourself to a particular release.
> 

Absolutely in the limit. But there are many bits of code that work quite
nicely from release to release because they use services that live in the
smooth water in the wake of the Linux head.

I think defining that smooth water has merit. I also think that it would be
nice to limit the scope of module externs to avoid polluting the global
namespace. I'm not sure that this particular patch reaches these goals, but
it prompted me to comment.

> So you're not proposing we mark what's not stable, you're arguing that we
> create a subset which is stable.
> 

Well, this is an interesting question. The answer is I think both are
important. It would be nice (and arguably necessary long term) to limit the
scope of externs. This can be accomplished with name spaces "I want bob's
implementation of read."

I think it also has value to define interfaces that are considered stable
(but not inviolate) to allow developers to make better informed decisions
when choosing interfaces. Having this info explicit in the code seems
logical to me.

> That's an argument we're not (yet) having.
> 

Yeah, maybe I'm off in the weeds on this one...

Tom

> Cheers,
> Rusty.
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Adrian Bunk]

On Wed, Nov 28, 2007 at 12:06:45AM +0100, Andi Kleen wrote:
> On Tue, Nov 27, 2007 at 03:00:22PM -0800, Stephen Hemminger wrote:
>...
> > of a modular ipv6 is flawed. 
> 
> Modules that cannot be unloaded are still useful. Standard case: Distributions
> like to offer an option to not use ipv6 because that is popular workaround
> for the common "DNS server eats AAAA queries and causes delays" issue.
> Forcing the user to rebuild the kernel for this wouldn't be practical.
> If ipv6 wasn't modular that would be hard to do.

It should be trivial doing it similar to the selinux=0 boot option.

> -Andi

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

On Wednesday 28 November 2007 17:48:17 Adrian Bunk wrote:
> On Wed, Nov 28, 2007 at 12:06:45AM +0100, Andi Kleen wrote:
> > On Tue, Nov 27, 2007 at 03:00:22PM -0800, Stephen Hemminger wrote:
> >...
> > > of a modular ipv6 is flawed. 
> > 
> > Modules that cannot be unloaded are still useful. Standard case: Distributions
> > like to offer an option to not use ipv6 because that is popular workaround
> > for the common "DNS server eats AAAA queries and causes delays" issue.
> > Forcing the user to rebuild the kernel for this wouldn't be practical.
> > If ipv6 wasn't modular that would be hard to do.
> 
> It should be trivial doing it similar to the selinux=0 boot option.

They safe also a few hundred KB of memory this way.
I know it is not en vogue anymore to care about memory bloat, but
I personally like that.

-Andi 

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Arnd Bergmann]

On Thursday 22 November 2007, Andi Kleen wrote:
>  #define EXPORT_SYMBOL(sym)                                     \
> -       __EXPORT_SYMBOL(sym, "")
> +       __EXPORT_SYMBOL(sym, "",,, NULL)
>  
>  #define EXPORT_SYMBOL_GPL(sym)                                 \
> -       __EXPORT_SYMBOL(sym, "_gpl")
> +       __EXPORT_SYMBOL(sym, "_gpl",,, NULL)
>  
>  #define EXPORT_SYMBOL_GPL_FUTURE(sym)                          \
> -       __EXPORT_SYMBOL(sym, "_gpl_future")
> +       __EXPORT_SYMBOL(sym, "_gpl_future",,, NULL)
>  
> +/* Export symbol into namespace ns
> + * No _GPL variants because namespaces imply GPL only
> + */
> +#define EXPORT_SYMBOL_NS(ns, sym)                              \
> +       __EXPORT_SYMBOL(sym, "_gpl",__##ns, NS_SEPARATOR #ns, #ns)
>  

I think it would be good if you could specify a default namespace
per module, that could reduce the amount of necessary changes significantly.

For example, you can do

#define EXPORT_SYMBOL_GLOBAL(sym) __EXPORT_SYMBOL(sym, "_gpl",,, NULL)
#ifdef MODULE_NAMESPACE
#define EXPORT_SYMBOL_GPL(sym) EXPORT_SYMBOL_GLOBAL(sym)
#else
#define EXPORT_SYMBOL_GPL(sym) EXPORT_SYMBOL_NS(sym, MODULE_NAMESPACE)
#endif

If we go that way, it may be useful to extend the namespace mechanism to
non-GPL symbols as well, like

#define EXPORT_SYMBOL(sym) __EXPORT_SYMBOL(sym, "",__## MODULE_NAMESPACE, NS_SEPARATOR #MODULE_NAMESPACE, #MODULE_NAMESPACE)

Unfortunately, doing this automatic namespace selection requires to set
the namespace before #include <linux/module.h>. One way to work around this
could be to use Makefile magic so you can list a Makefile as

obj-$(CONFIG_COMBINED) += combined.o
combined-$(CONFIG_SUBOPTION) += combined_main.o combined_other.o
obj-$(CONFIG_SINGLE) += single.o
obj-$(CONFIG_OTHER) += other.o
obj-$(CONFIG_API) += api.o

NAMESPACE = subsys                       # default, used for other.o
NAMESPACE_single.o = single              # used only for single.o
NAMESPACE_combined.o = combined          # all parts of combined.o
NAMESPACE_combined_other.o = special     #    except this one
NAMESPACE_api.o =                        # api.o is put into the global ns

The Makefile logic here would basically just follow the rules we have for
CFLAGS etc, and then pass -DMODULE_NAMESPACE=$(NAMESPACE_$(obj)) to gcc.

	Arnd <><

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Andi Kleen]

> I think it would be good if you could specify a default namespace
> per module, that could reduce the amount of necessary changes significantly.

But also give less documentation. It's also not that difficult to mark
the exports once. I've forward ported such patches over a few kernels
and didn't run into significant me

> obj-$(CONFIG_COMBINED) += combined.o
> combined-$(CONFIG_SUBOPTION) += combined_main.o combined_other.o
> obj-$(CONFIG_SINGLE) += single.o
> obj-$(CONFIG_OTHER) += other.o
> obj-$(CONFIG_API) += api.o
> 
> NAMESPACE = subsys                       # default, used for other.o
> NAMESPACE_single.o = single              # used only for single.o
> NAMESPACE_combined.o = combined          # all parts of combined.o
> NAMESPACE_combined_other.o = special     #    except this one
> NAMESPACE_api.o =                        # api.o is put into the global ns

I would prefer to keep that inside the source files, again for 
documentation purposes. One goal of namespace was to make something
that was previously kind of implicit explicit and the default name
spaces would work against that again I think.

-Andi
 

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Arnd Bergmann]

On Thursday 29 November 2007, Andi Kleen wrote:
> > I think it would be good if you could specify a default namespace
> > per module, that could reduce the amount of necessary changes significantly.
> 
> But also give less documentation. It's also not that difficult to mark
> the exports once. I've forward ported such patches over a few kernels
> and didn't run into significant me

Part of your sentence seems to be missing, but I guess I understand your
point. How many files did you annotate this way? I can see it as being
useful to have the namespace explicit in each symbol, but doing it once
per module sounds like the 80% solution for 20% of the work, and the
two don't even conflict. In the current kernel, I count 12644 exported
symbols in 1646 files, in 540 directories.

One problem I can see with annotating every symbol is that it conflicts
with other patches that add more exported functions to a file without
adding the namespace, or that simply break because of context changes.

	Arnd <><

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Arjan van de Ven]

On Mon, 26 Nov 2007 10:25:33 -0800
> 
> Agreed. On first glance, I was intrigued but:
> 
> 1) Why is everyone so concerned that export symbol space is large?
> 	- does it cost cpu or running memory?

yes. about 120 bytes per symbol

> 	- does it cause bugs?

yes, bad apis are causing bugs... sys_open is just the starter of that.


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.

[Rusty Russell]

On Friday 30 November 2007 03:53:34 Arjan van de Ven wrote:
> On Mon, 26 Nov 2007 10:25:33 -0800
>
> > Agreed. On first glance, I was intrigued but:
> >
> > 1) Why is everyone so concerned that export symbol space is large?
> > 	- does it cost cpu or running memory?
>
> yes. about 120 bytes per symbol

But this patch makes that worse, not better.

> > 	- does it cause bugs?
>
> yes, bad apis are causing bugs... sys_open is just the starter of that.

Sure, but this doesn't change the APIs, either.  We seem to have fixed 
sys_open the right way, and since we're not supposed to care about 
out-of-tree modules...

Rusty.

Re: [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like

[Sam Ravnborg]

On Thu, Nov 22, 2007 at 03:43:08AM +0100, Andi Kleen wrote:
> 
> This way gcc can warn for wrong format strings

This loks good. Can I get i s-o-b then I will apply it.

	Sam


> 
> ---
>  scripts/mod/modpost.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> Index: linux/scripts/mod/modpost.c
> ===================================================================
> --- linux.orig/scripts/mod/modpost.c
> +++ linux/scripts/mod/modpost.c
> @@ -33,7 +33,9 @@ enum export {
>  	export_unused_gpl, export_gpl_future, export_unknown
>  };
>  
> -void fatal(const char *fmt, ...)
> +#define PRINTF __attribute__ ((format (printf, 1, 2)))
> +
> +PRINTF void fatal(const char *fmt, ...)
>  {
>  	va_list arglist;
>  
> @@ -46,7 +48,7 @@ void fatal(const char *fmt, ...)
>  	exit(1);
>  }
>  
> -void warn(const char *fmt, ...)
> +PRINTF void warn(const char *fmt, ...)
>  {
>  	va_list arglist;
>  
> @@ -57,7 +59,7 @@ void warn(const char *fmt, ...)
>  	va_end(arglist);
>  }
>  
> -void merror(const char *fmt, ...)
> +PRINTF void merror(const char *fmt, ...)
>  {
>  	va_list arglist;
>  

Re: [PATCH RFC] [4/9] modpost: Fix format string warnings

[Sam Ravnborg]

On Thu, Nov 22, 2007 at 03:43:09AM +0100, Andi Kleen wrote:
> 
> Fix wrong format strings in modpost exposed by the previous patch.
> Including one missing argument -- some random data was printed instead.

Looks good. Can I get a s-o-b then I will apply it.

	Sam

> 
> ---
>  scripts/mod/modpost.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> Index: linux/scripts/mod/modpost.c
> ===================================================================
> --- linux.orig/scripts/mod/modpost.c
> +++ linux/scripts/mod/modpost.c
> @@ -388,7 +388,7 @@ static int parse_elf(struct elf_info *in
>  
>  	/* Check if file offset is correct */
>  	if (hdr->e_shoff > info->size) {
> -		fatal("section header offset=%u in file '%s' is bigger then filesize=%lu\n", hdr->e_shoff, filename, info->size);
> +		fatal("section header offset=%lu in file '%s' is bigger then filesize=%lu\n", (unsigned long)hdr->e_shoff, filename, info->size);
>  		return 0;
>  	}
>  
> @@ -409,7 +409,7 @@ static int parse_elf(struct elf_info *in
>  		const char *secname;
>  
>  		if (sechdrs[i].sh_offset > info->size) {
> -			fatal("%s is truncated. sechdrs[i].sh_offset=%u > sizeof(*hrd)=%ul\n", filename, (unsigned int)sechdrs[i].sh_offset, sizeof(*hdr));
> +			fatal("%s is truncated. sechdrs[i].sh_offset=%lu > sizeof(*hrd)=%lu\n", filename, (unsigned long)sechdrs[i].sh_offset, sizeof(*hdr));
>  			return 0;
>  		}
>  		secname = secstrings + sechdrs[i].sh_name;
> @@ -907,7 +907,8 @@ static void warn_sec_mismatch(const char
>  		     "before '%s' (at offset -0x%llx)\n",
>  		     modname, fromsec, (unsigned long long)r.r_offset,
>  		     secname, refsymname,
> -		     elf->strtab + after->st_name);
> +		     elf->strtab + after->st_name,
> +		     (unsigned long long)r.r_offset);
>  	} else {
>  		warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s\n",
>  		     modname, fromsec, (unsigned long long)r.r_offset,

Re: [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like

[Andi Kleen]

On Mon, Dec 10, 2007 at 07:50:08PM +0100, Sam Ravnborg wrote:
> On Thu, Nov 22, 2007 at 03:43:08AM +0100, Andi Kleen wrote:
> > 
> > This way gcc can warn for wrong format strings
> 
> This loks good. Can I get i s-o-b then I will apply it.

Sorry must have been left out by mistake.

Signed-off-by: Andi Kleen <ak@suse.de>

for both patches.

-Andi

Re: [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost

[Sam Ravnborg]

On Thu, Nov 22, 2007 at 03:43:10AM +0100, Andi Kleen wrote:
> 
> When passing an file name > 1k the stack could be overflowed.
> Not really a security issue, but still better plugged.

Looks good. A s-o-b line again please.
Although I am not so happy with the ue of gcc extensions.

	Sam
> 
> 
> ---
>  scripts/mod/modpost.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Index: linux/scripts/mod/modpost.c
> ===================================================================
> --- linux.orig/scripts/mod/modpost.c
> +++ linux/scripts/mod/modpost.c
> @@ -1656,7 +1656,6 @@ int main(int argc, char **argv)
>  {
>  	struct module *mod;
>  	struct buffer buf = { };
> -	char fname[SZ];
>  	char *kernel_read = NULL, *module_read = NULL;
>  	char *dump_write = NULL;
>  	int opt;
> @@ -1709,6 +1708,8 @@ int main(int argc, char **argv)
>  	err = 0;
>  
>  	for (mod = modules; mod; mod = mod->next) {
> +		char fname[strlen(mod->name) + 10];
> +
>  		if (mod->skip)
>  			continue;
>  

Re: [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost

[Andi Kleen]

On Monday 10 December 2007 20:32, Sam Ravnborg wrote:
> On Thu, Nov 22, 2007 at 03:43:10AM +0100, Andi Kleen wrote:
> > When passing an file name > 1k the stack could be overflowed.
> > Not really a security issue, but still better plugged.
>
> Looks good. A s-o-b line again please.

Signed-off-by: Andi Kleen <ak@suse.de>

> Although I am not so happy with the ue of gcc extensions.

That's not a gcc extension. It's C99.

-Andi

Re: [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost

[Sam Ravnborg]

On Mon, Dec 10, 2007 at 08:57:28PM +0100, Andi Kleen wrote:
> On Monday 10 December 2007 20:32, Sam Ravnborg wrote:
> > On Thu, Nov 22, 2007 at 03:43:10AM +0100, Andi Kleen wrote:
> > > When passing an file name > 1k the stack could be overflowed.
> > > Not really a security issue, but still better plugged.
> >
> > Looks good. A s-o-b line again please.
> 
> Signed-off-by: Andi Kleen <ak@suse.de>
> 
> > Although I am not so happy with the ue of gcc extensions.
> 
> That's not a gcc extension. It's C99.

OK.
I have applied all three patches to kbuild.git.

As I did not follow the whole thread about the namespace I did not
take those.
And the first patch touching module.c should go in via akpm I think.
It is outside my core-competence area at least .

	Sam

Re: [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like

[Rusty Russell]

On Tuesday 11 December 2007 05:50:08 Sam Ravnborg wrote:
> On Thu, Nov 22, 2007 at 03:43:08AM +0100, Andi Kleen wrote:
> > This way gcc can warn for wrong format strings
>
> This loks good. Can I get i s-o-b then I will apply it.

Even better, switch the code to the standard warn/warnx/err/errx.

Cheers,
Rusty.