* [PATCH RFC] [2/9] Fix duplicate symbol check to also check future gpl and unused symbols
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
@ 2007-11-22 2:43 ` Andi Kleen
2007-11-22 2:43 ` [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like Andi Kleen
` (13 subsequent siblings)
14 siblings, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, sam, rusty
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. */
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
2007-11-22 2:43 ` [PATCH RFC] [2/9] Fix duplicate symbol check to also check future gpl and unused symbols Andi Kleen
@ 2007-11-22 2:43 ` Andi Kleen
2007-12-10 18:50 ` Sam Ravnborg
2007-11-22 2:43 ` [PATCH RFC] [4/9] modpost: Fix format string warnings Andi Kleen
` (12 subsequent siblings)
14 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, sam, rusty
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;
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like
2007-11-22 2:43 ` [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like Andi Kleen
@ 2007-12-10 18:50 ` Sam Ravnborg
2007-12-10 18:58 ` Andi Kleen
2007-12-12 1:37 ` Rusty Russell
0 siblings, 2 replies; 83+ messages in thread
From: Sam Ravnborg @ 2007-12-10 18:50 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, rusty
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;
>
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like
2007-12-10 18:50 ` Sam Ravnborg
@ 2007-12-10 18:58 ` Andi Kleen
2007-12-12 1:37 ` Rusty Russell
1 sibling, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-12-10 18:58 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: Andi Kleen, netdev, linux-kernel, rusty
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like
2007-12-10 18:50 ` Sam Ravnborg
2007-12-10 18:58 ` Andi Kleen
@ 2007-12-12 1:37 ` Rusty Russell
1 sibling, 0 replies; 83+ messages in thread
From: Rusty Russell @ 2007-12-12 1:37 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: Andi Kleen, netdev, linux-kernel
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH RFC] [4/9] modpost: Fix format string warnings
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
2007-11-22 2:43 ` [PATCH RFC] [2/9] Fix duplicate symbol check to also check future gpl and unused symbols Andi Kleen
2007-11-22 2:43 ` [PATCH RFC] [3/9] modpost: Declare the modpost error functions as printf like Andi Kleen
@ 2007-11-22 2:43 ` Andi Kleen
2007-12-10 18:50 ` Sam Ravnborg
2007-11-22 2:43 ` [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost Andi Kleen
` (11 subsequent siblings)
14 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, sam, rusty
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,
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [4/9] modpost: Fix format string warnings
2007-11-22 2:43 ` [PATCH RFC] [4/9] modpost: Fix format string warnings Andi Kleen
@ 2007-12-10 18:50 ` Sam Ravnborg
0 siblings, 0 replies; 83+ messages in thread
From: Sam Ravnborg @ 2007-12-10 18:50 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, rusty
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,
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (2 preceding siblings ...)
2007-11-22 2:43 ` [PATCH RFC] [4/9] modpost: Fix format string warnings Andi Kleen
@ 2007-11-22 2:43 ` Andi Kleen
2007-12-10 19:32 ` Sam Ravnborg
2007-11-22 2:43 ` [PATCH RFC] [6/9] Implement namespace checking " Andi Kleen
` (10 subsequent siblings)
14 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, sam, rusty
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;
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost
2007-11-22 2:43 ` [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost Andi Kleen
@ 2007-12-10 19:32 ` Sam Ravnborg
2007-12-10 19:57 ` Andi Kleen
0 siblings, 1 reply; 83+ messages in thread
From: Sam Ravnborg @ 2007-12-10 19:32 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, rusty
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;
>
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost
2007-12-10 19:32 ` Sam Ravnborg
@ 2007-12-10 19:57 ` Andi Kleen
2007-12-10 20:07 ` Sam Ravnborg
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-12-10 19:57 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: netdev, linux-kernel, rusty
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost
2007-12-10 19:57 ` Andi Kleen
@ 2007-12-10 20:07 ` Sam Ravnborg
0 siblings, 0 replies; 83+ messages in thread
From: Sam Ravnborg @ 2007-12-10 20:07 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, rusty
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH RFC] [6/9] Implement namespace checking in modpost
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (3 preceding siblings ...)
2007-11-22 2:43 ` [PATCH RFC] [5/9] modpost: Fix a buffer overflow in modpost Andi Kleen
@ 2007-11-22 2:43 ` Andi Kleen
2007-11-22 2:43 ` [PATCH RFC] [7/9] Convert TCP exports into namespaces Andi Kleen
` (9 subsequent siblings)
14 siblings, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, sam, rusty
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) {
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH RFC] [7/9] Convert TCP exports into namespaces
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (4 preceding siblings ...)
2007-11-22 2:43 ` [PATCH RFC] [6/9] Implement namespace checking " Andi Kleen
@ 2007-11-22 2:43 ` Andi Kleen
2007-11-22 2:43 ` [PATCH RFC] [8/9] Put UDP exports into a namespace Andi Kleen
` (8 subsequent siblings)
14 siblings, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, sam, rusty
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);
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH RFC] [8/9] Put UDP exports into a namespace
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (5 preceding siblings ...)
2007-11-22 2:43 ` [PATCH RFC] [7/9] Convert TCP exports into namespaces Andi Kleen
@ 2007-11-22 2:43 ` Andi Kleen
2007-11-22 2:43 ` [PATCH RFC] [9/9] Add a inet namespace Andi Kleen
` (7 subsequent siblings)
14 siblings, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, sam, rusty
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);
^ permalink raw reply [flat|nested] 83+ messages in thread
* [PATCH RFC] [9/9] Add a inet namespace
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (6 preceding siblings ...)
2007-11-22 2:43 ` [PATCH RFC] [8/9] Put UDP exports into a namespace Andi Kleen
@ 2007-11-22 2:43 ` Andi Kleen
2007-11-22 3:03 ` [PATCH RFC] [1/9] Core module symbol namespaces code and intro Arjan van de Ven
` (6 subsequent siblings)
14 siblings, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 2:43 UTC (permalink / raw)
To: netdev, linux-kernel, sam, rusty
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);
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (7 preceding siblings ...)
2007-11-22 2:43 ` [PATCH RFC] [9/9] Add a inet namespace Andi Kleen
@ 2007-11-22 3:03 ` Arjan van de Ven
2007-11-22 3:37 ` Andi Kleen
2007-11-22 3:52 ` Dave Jones
` (5 subsequent siblings)
14 siblings, 1 reply; 83+ messages in thread
From: Arjan van de Ven @ 2007-11-22 3:03 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam, rusty
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 3:03 ` [PATCH RFC] [1/9] Core module symbol namespaces code and intro Arjan van de Ven
@ 2007-11-22 3:37 ` Andi Kleen
0 siblings, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 3:37 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: netdev, linux-kernel, sam, rusty
> 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (8 preceding siblings ...)
2007-11-22 3:03 ` [PATCH RFC] [1/9] Core module symbol namespaces code and intro Arjan van de Ven
@ 2007-11-22 3:52 ` Dave Jones
2007-11-22 3:56 ` Rusty Russell
` (4 subsequent siblings)
14 siblings, 0 replies; 83+ messages in thread
From: Dave Jones @ 2007-11-22 3:52 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam, rusty
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (9 preceding siblings ...)
2007-11-22 3:52 ` Dave Jones
@ 2007-11-22 3:56 ` Rusty Russell
2007-11-22 8:41 ` Dave Young
` (2 more replies)
2007-11-22 11:06 ` Christoph Hellwig
` (3 subsequent siblings)
14 siblings, 3 replies; 83+ messages in thread
From: Rusty Russell @ 2007-11-22 3:56 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 3:56 ` Rusty Russell
@ 2007-11-22 8:41 ` Dave Young
2007-11-22 18:19 ` Andi Kleen
2007-11-22 11:05 ` Christoph Hellwig
2007-11-22 11:46 ` Andi Kleen
2 siblings, 1 reply; 83+ messages in thread
From: Dave Young @ 2007-11-22 8:41 UTC (permalink / raw)
To: Rusty Russell; +Cc: Andi Kleen, netdev, linux-kernel, sam
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/
>
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 8:41 ` Dave Young
@ 2007-11-22 18:19 ` Andi Kleen
2007-11-23 2:06 ` Dave Young
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 18:19 UTC (permalink / raw)
To: Dave Young; +Cc: Rusty Russell, netdev, linux-kernel, sam
> 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 18:19 ` Andi Kleen
@ 2007-11-23 2:06 ` Dave Young
0 siblings, 0 replies; 83+ messages in thread
From: Dave Young @ 2007-11-23 2:06 UTC (permalink / raw)
To: Andi Kleen; +Cc: Rusty Russell, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 3:56 ` Rusty Russell
2007-11-22 8:41 ` Dave Young
@ 2007-11-22 11:05 ` Christoph Hellwig
2007-11-23 0:25 ` Rusty Russell
2007-11-22 11:46 ` Andi Kleen
2 siblings, 1 reply; 83+ messages in thread
From: Christoph Hellwig @ 2007-11-22 11:05 UTC (permalink / raw)
To: Rusty Russell; +Cc: Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 11:05 ` Christoph Hellwig
@ 2007-11-23 0:25 ` Rusty Russell
2007-11-23 1:36 ` Andi Kleen
0 siblings, 1 reply; 83+ messages in thread
From: Rusty Russell @ 2007-11-23 0:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-23 0:25 ` Rusty Russell
@ 2007-11-23 1:36 ` Andi Kleen
2007-11-23 3:35 ` Rusty Russell
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-23 1:36 UTC (permalink / raw)
To: Rusty Russell; +Cc: Christoph Hellwig, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-23 1:36 ` Andi Kleen
@ 2007-11-23 3:35 ` Rusty Russell
2007-11-23 19:53 ` Andi Kleen
0 siblings, 1 reply; 83+ messages in thread
From: Rusty Russell @ 2007-11-23 3:35 UTC (permalink / raw)
To: Andi Kleen; +Cc: Christoph Hellwig, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-23 3:35 ` Rusty Russell
@ 2007-11-23 19:53 ` Andi Kleen
2007-11-24 4:53 ` Rusty Russell
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-23 19:53 UTC (permalink / raw)
To: Rusty Russell; +Cc: Andi Kleen, Christoph Hellwig, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-23 19:53 ` Andi Kleen
@ 2007-11-24 4:53 ` Rusty Russell
2007-11-24 12:39 ` Andi Kleen
0 siblings, 1 reply; 83+ messages in thread
From: Rusty Russell @ 2007-11-24 4:53 UTC (permalink / raw)
To: Andi Kleen; +Cc: Andi Kleen, Christoph Hellwig, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-24 4:53 ` Rusty Russell
@ 2007-11-24 12:39 ` Andi Kleen
2007-11-26 1:23 ` Rusty Russell
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-24 12:39 UTC (permalink / raw)
To: Rusty Russell
Cc: Andi Kleen, Andi Kleen, Christoph Hellwig, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-24 12:39 ` Andi Kleen
@ 2007-11-26 1:23 ` Rusty Russell
0 siblings, 0 replies; 83+ messages in thread
From: Rusty Russell @ 2007-11-26 1:23 UTC (permalink / raw)
To: Andi Kleen; +Cc: Andi Kleen, Christoph Hellwig, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 3:56 ` Rusty Russell
2007-11-22 8:41 ` Dave Young
2007-11-22 11:05 ` Christoph Hellwig
@ 2007-11-22 11:46 ` Andi Kleen
2007-11-23 0:29 ` Rusty Russell
2 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 11:46 UTC (permalink / raw)
To: Rusty Russell; +Cc: netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 11:46 ` Andi Kleen
@ 2007-11-23 0:29 ` Rusty Russell
2007-11-25 20:29 ` Roland Dreier
0 siblings, 1 reply; 83+ messages in thread
From: Rusty Russell @ 2007-11-23 0:29 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-23 0:29 ` Rusty Russell
@ 2007-11-25 20:29 ` Roland Dreier
2007-11-26 1:25 ` Rusty Russell
0 siblings, 1 reply; 83+ messages in thread
From: Roland Dreier @ 2007-11-25 20:29 UTC (permalink / raw)
To: Rusty Russell; +Cc: Andi Kleen, netdev, linux-kernel, sam
> 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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-25 20:29 ` Roland Dreier
@ 2007-11-26 1:25 ` Rusty Russell
2007-11-26 5:58 ` Roland Dreier
0 siblings, 1 reply; 83+ messages in thread
From: Rusty Russell @ 2007-11-26 1:25 UTC (permalink / raw)
To: Roland Dreier; +Cc: Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-26 1:25 ` Rusty Russell
@ 2007-11-26 5:58 ` Roland Dreier
2007-11-27 4:26 ` Rusty Russell
0 siblings, 1 reply; 83+ messages in thread
From: Roland Dreier @ 2007-11-26 5:58 UTC (permalink / raw)
To: Rusty Russell; +Cc: Andi Kleen, netdev, linux-kernel, sam
> > 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...
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-26 5:58 ` Roland Dreier
@ 2007-11-27 4:26 ` Rusty Russell
2007-11-27 10:50 ` Andi Kleen
0 siblings, 1 reply; 83+ messages in thread
From: Rusty Russell @ 2007-11-27 4:26 UTC (permalink / raw)
To: Roland Dreier; +Cc: Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 4:26 ` Rusty Russell
@ 2007-11-27 10:50 ` Andi Kleen
2007-11-27 13:58 ` Herbert Xu
2007-11-28 1:34 ` Rusty Russell
0 siblings, 2 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-27 10:50 UTC (permalink / raw)
To: Rusty Russell; +Cc: Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 10:50 ` Andi Kleen
@ 2007-11-27 13:58 ` Herbert Xu
2007-11-27 14:12 ` Andi Kleen
2007-11-28 1:34 ` Rusty Russell
1 sibling, 1 reply; 83+ messages in thread
From: Herbert Xu @ 2007-11-27 13:58 UTC (permalink / raw)
To: Andi Kleen; +Cc: rusty, rdreier, ak, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 13:58 ` Herbert Xu
@ 2007-11-27 14:12 ` Andi Kleen
2007-11-27 14:36 ` Herbert Xu
2007-11-27 20:02 ` Valdis.Kletnieks
0 siblings, 2 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-27 14:12 UTC (permalink / raw)
To: Herbert Xu; +Cc: Andi Kleen, rusty, rdreier, ak, netdev, linux-kernel, sam
> 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 14:12 ` Andi Kleen
@ 2007-11-27 14:36 ` Herbert Xu
2007-11-27 20:02 ` Valdis.Kletnieks
1 sibling, 0 replies; 83+ messages in thread
From: Herbert Xu @ 2007-11-27 14:36 UTC (permalink / raw)
To: Andi Kleen; +Cc: rusty, rdreier, ak, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 14:12 ` Andi Kleen
2007-11-27 14:36 ` Herbert Xu
@ 2007-11-27 20:02 ` Valdis.Kletnieks
1 sibling, 0 replies; 83+ messages in thread
From: Valdis.Kletnieks @ 2007-11-27 20:02 UTC (permalink / raw)
To: Andi Kleen; +Cc: Herbert Xu, rusty, rdreier, ak, netdev, linux-kernel, sam
[-- 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 --]
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 10:50 ` Andi Kleen
2007-11-27 13:58 ` Herbert Xu
@ 2007-11-28 1:34 ` Rusty Russell
1 sibling, 0 replies; 83+ messages in thread
From: Rusty Russell @ 2007-11-28 1:34 UTC (permalink / raw)
To: Andi Kleen; +Cc: Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (10 preceding siblings ...)
2007-11-22 3:56 ` Rusty Russell
@ 2007-11-22 11:06 ` Christoph Hellwig
2007-11-22 11:54 ` Andi Kleen
2007-11-22 12:01 ` Arnaldo Carvalho de Melo
` (2 subsequent siblings)
14 siblings, 1 reply; 83+ messages in thread
From: Christoph Hellwig @ 2007-11-22 11:06 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam, rusty
Very nice, looking forward to organize the exports mess a bit more.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 11:06 ` Christoph Hellwig
@ 2007-11-22 11:54 ` Andi Kleen
2007-11-22 12:03 ` Christoph Hellwig
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 11:54 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: netdev, linux-kernel, sam, rusty
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 11:54 ` Andi Kleen
@ 2007-11-22 12:03 ` Christoph Hellwig
0 siblings, 0 replies; 83+ messages in thread
From: Christoph Hellwig @ 2007-11-22 12:03 UTC (permalink / raw)
To: Andi Kleen; +Cc: Christoph Hellwig, netdev, linux-kernel, sam, rusty
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (11 preceding siblings ...)
2007-11-22 11:06 ` Christoph Hellwig
@ 2007-11-22 12:01 ` Arnaldo Carvalho de Melo
2007-11-22 18:17 ` Andi Kleen
2007-11-25 20:27 ` Roland Dreier
2007-11-29 9:55 ` Arnd Bergmann
14 siblings, 1 reply; 83+ messages in thread
From: Arnaldo Carvalho de Melo @ 2007-11-22 12:01 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam, rusty
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 12:01 ` Arnaldo Carvalho de Melo
@ 2007-11-22 18:17 ` Andi Kleen
0 siblings, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-22 18:17 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: netdev, linux-kernel, sam, rusty
> 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (12 preceding siblings ...)
2007-11-22 12:01 ` Arnaldo Carvalho de Melo
@ 2007-11-25 20:27 ` Roland Dreier
2007-11-26 1:28 ` Rusty Russell
2007-11-29 9:55 ` Arnd Bergmann
14 siblings, 1 reply; 83+ messages in thread
From: Roland Dreier @ 2007-11-25 20:27 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam, rusty
> 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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-25 20:27 ` Roland Dreier
@ 2007-11-26 1:28 ` Rusty Russell
2007-11-26 6:15 ` Roland Dreier
2007-11-26 18:25 ` Stephen Hemminger
0 siblings, 2 replies; 83+ messages in thread
From: Rusty Russell @ 2007-11-26 1:28 UTC (permalink / raw)
To: Roland Dreier; +Cc: Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-26 1:28 ` Rusty Russell
@ 2007-11-26 6:15 ` Roland Dreier
2007-11-27 4:49 ` Rusty Russell
2007-11-26 18:25 ` Stephen Hemminger
1 sibling, 1 reply; 83+ messages in thread
From: Roland Dreier @ 2007-11-26 6:15 UTC (permalink / raw)
To: Rusty Russell; +Cc: Andi Kleen, netdev, linux-kernel, sam
> 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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-26 6:15 ` Roland Dreier
@ 2007-11-27 4:49 ` Rusty Russell
2007-11-27 5:35 ` Tom Tucker
` (2 more replies)
0 siblings, 3 replies; 83+ messages in thread
From: Rusty Russell @ 2007-11-27 4:49 UTC (permalink / raw)
To: Roland Dreier; +Cc: Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 4:49 ` Rusty Russell
@ 2007-11-27 5:35 ` Tom Tucker
2007-11-27 9:02 ` Andi Kleen
` (2 more replies)
2007-11-27 15:43 ` Jonathan Corbet
2007-11-27 22:31 ` Jon Masters
2 siblings, 3 replies; 83+ messages in thread
From: Tom Tucker @ 2007-11-27 5:35 UTC (permalink / raw)
To: Rusty Russell; +Cc: Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 5:35 ` Tom Tucker
@ 2007-11-27 9:02 ` Andi Kleen
2007-11-27 17:24 ` Adrian Bunk
2007-11-27 17:15 ` Adrian Bunk
2007-11-28 1:27 ` Rusty Russell
2 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-27 9:02 UTC (permalink / raw)
To: Tom Tucker; +Cc: Rusty Russell, Roland Dreier, netdev, linux-kernel, sam
> > 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 9:02 ` Andi Kleen
@ 2007-11-27 17:24 ` Adrian Bunk
0 siblings, 0 replies; 83+ messages in thread
From: Adrian Bunk @ 2007-11-27 17:24 UTC (permalink / raw)
To: Andi Kleen
Cc: Tom Tucker, Rusty Russell, Roland Dreier, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 5:35 ` Tom Tucker
2007-11-27 9:02 ` Andi Kleen
@ 2007-11-27 17:15 ` Adrian Bunk
2007-11-27 17:45 ` Tom Tucker
2007-11-28 1:27 ` Rusty Russell
2 siblings, 1 reply; 83+ messages in thread
From: Adrian Bunk @ 2007-11-27 17:15 UTC (permalink / raw)
To: Tom Tucker
Cc: Rusty Russell, Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 17:15 ` Adrian Bunk
@ 2007-11-27 17:45 ` Tom Tucker
2007-11-27 18:59 ` Adrian Bunk
0 siblings, 1 reply; 83+ messages in thread
From: Tom Tucker @ 2007-11-27 17:45 UTC (permalink / raw)
To: Adrian Bunk
Cc: Rusty Russell, Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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
>
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 17:45 ` Tom Tucker
@ 2007-11-27 18:59 ` Adrian Bunk
0 siblings, 0 replies; 83+ messages in thread
From: Adrian Bunk @ 2007-11-27 18:59 UTC (permalink / raw)
To: Tom Tucker
Cc: Rusty Russell, Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 5:35 ` Tom Tucker
2007-11-27 9:02 ` Andi Kleen
2007-11-27 17:15 ` Adrian Bunk
@ 2007-11-28 1:27 ` Rusty Russell
2007-11-28 4:12 ` Tom Tucker
2 siblings, 1 reply; 83+ messages in thread
From: Rusty Russell @ 2007-11-28 1:27 UTC (permalink / raw)
To: Tom Tucker; +Cc: Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-28 1:27 ` Rusty Russell
@ 2007-11-28 4:12 ` Tom Tucker
0 siblings, 0 replies; 83+ messages in thread
From: Tom Tucker @ 2007-11-28 4:12 UTC (permalink / raw)
To: Rusty Russell; +Cc: Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 4:49 ` Rusty Russell
2007-11-27 5:35 ` Tom Tucker
@ 2007-11-27 15:43 ` Jonathan Corbet
2007-11-27 15:58 ` Andi Kleen
2007-11-27 16:33 ` Christoph Hellwig
2007-11-27 22:31 ` Jon Masters
2 siblings, 2 replies; 83+ messages in thread
From: Jonathan Corbet @ 2007-11-27 15:43 UTC (permalink / raw)
To: Rusty Russell; +Cc: Andi Kleen, netdev, linux-kernel, sam, Roland Dreier
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 15:43 ` Jonathan Corbet
@ 2007-11-27 15:58 ` Andi Kleen
2007-11-27 16:33 ` Christoph Hellwig
1 sibling, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-27 15:58 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Rusty Russell, Andi Kleen, netdev, linux-kernel, sam, Roland Dreier
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 15:43 ` Jonathan Corbet
2007-11-27 15:58 ` Andi Kleen
@ 2007-11-27 16:33 ` Christoph Hellwig
1 sibling, 0 replies; 83+ messages in thread
From: Christoph Hellwig @ 2007-11-27 16:33 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Rusty Russell, Andi Kleen, netdev, linux-kernel, sam, Roland Dreier
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 4:49 ` Rusty Russell
2007-11-27 5:35 ` Tom Tucker
2007-11-27 15:43 ` Jonathan Corbet
@ 2007-11-27 22:31 ` Jon Masters
2007-11-27 22:37 ` Andi Kleen
2 siblings, 1 reply; 83+ messages in thread
From: Jon Masters @ 2007-11-27 22:31 UTC (permalink / raw)
To: Rusty Russell; +Cc: Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 22:31 ` Jon Masters
@ 2007-11-27 22:37 ` Andi Kleen
2007-11-27 23:00 ` Stephen Hemminger
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-27 22:37 UTC (permalink / raw)
To: Jon Masters
Cc: Rusty Russell, Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
> 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 22:37 ` Andi Kleen
@ 2007-11-27 23:00 ` Stephen Hemminger
2007-11-27 23:06 ` Andi Kleen
0 siblings, 1 reply; 83+ messages in thread
From: Stephen Hemminger @ 2007-11-27 23:00 UTC (permalink / raw)
To: Andi Kleen
Cc: Jon Masters, Rusty Russell, Roland Dreier, Andi Kleen, netdev,
linux-kernel, sam
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>
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 23:00 ` Stephen Hemminger
@ 2007-11-27 23:06 ` Andi Kleen
2007-11-28 16:48 ` Adrian Bunk
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-27 23:06 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Andi Kleen, Jon Masters, Rusty Russell, Roland Dreier,
Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 23:06 ` Andi Kleen
@ 2007-11-28 16:48 ` Adrian Bunk
2007-11-28 17:31 ` Andi Kleen
0 siblings, 1 reply; 83+ messages in thread
From: Adrian Bunk @ 2007-11-28 16:48 UTC (permalink / raw)
To: Andi Kleen
Cc: Stephen Hemminger, Jon Masters, Rusty Russell, Roland Dreier,
Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-28 16:48 ` Adrian Bunk
@ 2007-11-28 17:31 ` Andi Kleen
0 siblings, 0 replies; 83+ messages in thread
From: Andi Kleen @ 2007-11-28 17:31 UTC (permalink / raw)
To: Adrian Bunk
Cc: Stephen Hemminger, Jon Masters, Rusty Russell, Roland Dreier,
netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-26 1:28 ` Rusty Russell
2007-11-26 6:15 ` Roland Dreier
@ 2007-11-26 18:25 ` Stephen Hemminger
2007-11-26 22:18 ` Roland Dreier
` (2 more replies)
1 sibling, 3 replies; 83+ messages in thread
From: Stephen Hemminger @ 2007-11-26 18:25 UTC (permalink / raw)
To: Rusty Russell; +Cc: Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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>
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-26 18:25 ` Stephen Hemminger
@ 2007-11-26 22:18 ` Roland Dreier
2007-11-27 19:00 ` Dave Jones
2007-11-29 16:53 ` Arjan van de Ven
2 siblings, 0 replies; 83+ messages in thread
From: Roland Dreier @ 2007-11-26 22:18 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Rusty Russell, Andi Kleen, netdev, linux-kernel, sam
> 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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-26 18:25 ` Stephen Hemminger
2007-11-26 22:18 ` Roland Dreier
@ 2007-11-27 19:00 ` Dave Jones
2007-11-27 21:09 ` Adrian Bunk
2007-11-29 16:53 ` Arjan van de Ven
2 siblings, 1 reply; 83+ messages in thread
From: Dave Jones @ 2007-11-27 19:00 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Rusty Russell, Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 19:00 ` Dave Jones
@ 2007-11-27 21:09 ` Adrian Bunk
2007-11-27 21:15 ` Rick Jones
` (2 more replies)
0 siblings, 3 replies; 83+ messages in thread
From: Adrian Bunk @ 2007-11-27 21:09 UTC (permalink / raw)
To: Dave Jones, Stephen Hemminger, Rusty Russell, Roland Dreier,
Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 21:09 ` Adrian Bunk
@ 2007-11-27 21:15 ` Rick Jones
2007-11-27 21:38 ` Adrian Bunk
2007-11-27 21:25 ` Dave Jones
2007-11-27 23:00 ` Valdis.Kletnieks
2 siblings, 1 reply; 83+ messages in thread
From: Rick Jones @ 2007-11-27 21:15 UTC (permalink / raw)
To: Adrian Bunk
Cc: Dave Jones, Stephen Hemminger, Rusty Russell, Roland Dreier,
Andi Kleen, netdev, linux-kernel, sam
> 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 21:15 ` Rick Jones
@ 2007-11-27 21:38 ` Adrian Bunk
2007-11-28 2:01 ` Rick Jones
0 siblings, 1 reply; 83+ messages in thread
From: Adrian Bunk @ 2007-11-27 21:38 UTC (permalink / raw)
To: Rick Jones
Cc: Dave Jones, Stephen Hemminger, Rusty Russell, Roland Dreier,
Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 21:38 ` Adrian Bunk
@ 2007-11-28 2:01 ` Rick Jones
0 siblings, 0 replies; 83+ messages in thread
From: Rick Jones @ 2007-11-28 2:01 UTC (permalink / raw)
To: Adrian Bunk
Cc: Dave Jones, Stephen Hemminger, Rusty Russell, Roland Dreier,
Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 21:09 ` Adrian Bunk
2007-11-27 21:15 ` Rick Jones
@ 2007-11-27 21:25 ` Dave Jones
2007-11-27 23:00 ` Valdis.Kletnieks
2 siblings, 0 replies; 83+ messages in thread
From: Dave Jones @ 2007-11-27 21:25 UTC (permalink / raw)
To: Adrian Bunk
Cc: Stephen Hemminger, Rusty Russell, Roland Dreier, Andi Kleen,
netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-27 21:09 ` Adrian Bunk
2007-11-27 21:15 ` Rick Jones
2007-11-27 21:25 ` Dave Jones
@ 2007-11-27 23:00 ` Valdis.Kletnieks
2 siblings, 0 replies; 83+ messages in thread
From: Valdis.Kletnieks @ 2007-11-27 23:00 UTC (permalink / raw)
To: Adrian Bunk
Cc: Dave Jones, Stephen Hemminger, Rusty Russell, Roland Dreier,
Andi Kleen, netdev, linux-kernel, sam
[-- 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 --]
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-26 18:25 ` Stephen Hemminger
2007-11-26 22:18 ` Roland Dreier
2007-11-27 19:00 ` Dave Jones
@ 2007-11-29 16:53 ` Arjan van de Ven
2007-11-30 2:18 ` Rusty Russell
2 siblings, 1 reply; 83+ messages in thread
From: Arjan van de Ven @ 2007-11-29 16:53 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Rusty Russell, Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-29 16:53 ` Arjan van de Ven
@ 2007-11-30 2:18 ` Rusty Russell
0 siblings, 0 replies; 83+ messages in thread
From: Rusty Russell @ 2007-11-30 2:18 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Stephen Hemminger, Roland Dreier, Andi Kleen, netdev, linux-kernel, sam
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.
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-22 2:43 [PATCH RFC] [1/9] Core module symbol namespaces code and intro Andi Kleen
` (13 preceding siblings ...)
2007-11-25 20:27 ` Roland Dreier
@ 2007-11-29 9:55 ` Arnd Bergmann
2007-11-29 10:00 ` Andi Kleen
14 siblings, 1 reply; 83+ messages in thread
From: Arnd Bergmann @ 2007-11-29 9:55 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam, rusty
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 <><
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-29 9:55 ` Arnd Bergmann
@ 2007-11-29 10:00 ` Andi Kleen
2007-11-29 10:21 ` Arnd Bergmann
0 siblings, 1 reply; 83+ messages in thread
From: Andi Kleen @ 2007-11-29 10:00 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: netdev, linux-kernel, sam, rusty
> 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
^ permalink raw reply [flat|nested] 83+ messages in thread
* Re: [PATCH RFC] [1/9] Core module symbol namespaces code and intro.
2007-11-29 10:00 ` Andi Kleen
@ 2007-11-29 10:21 ` Arnd Bergmann
0 siblings, 0 replies; 83+ messages in thread
From: Arnd Bergmann @ 2007-11-29 10:21 UTC (permalink / raw)
To: Andi Kleen; +Cc: netdev, linux-kernel, sam, rusty
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 <><
^ permalink raw reply [flat|nested] 83+ messages in thread