[RFC][PATCH -mm take4 1/6] marking __init

[RFC][PATCH -mm take4 1/6] marking __init

[Keiichi KII]

From: Keiichi KII <k-keiichi@bx.jp.nec.com>

This patch contains the following cleanups.
 - add __init for initialization functions(option_setup() and
   init_netconsole()).

Acked-by: Matt Mackall <mpm@selenic.com>
Signed-off-by: Keiichi KII <k-keiichi@bx.jp.nec.com>
Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
---
Index: linux-mm/drivers/net/netconsole.c
===================================================================
--- linux-mm.orig/drivers/net/netconsole.c
+++ linux-mm/drivers/net/netconsole.c
@@ -91,7 +91,7 @@ static struct console netconsole = {
 	.write = write_msg
 };
 
-static int option_setup(char *opt)
+static int __init option_setup(char *opt)
 {
 	configured = !netpoll_parse_options(&np, opt);
 	return 1;
@@ -99,7 +99,7 @@ static int option_setup(char *opt)
 
 __setup("netconsole=", option_setup);
 
-static int init_netconsole(void)
+static int __init init_netconsole(void)
 {
 	int err;
 

-- 

[RFC][PATCH -mm take4 2/6] support multiple logging

[Keiichi KII]

From: Keiichi KII <k-keiichi@bx.jp.nec.com>

This patch contains the following changes for supporting multiple logging
 agents.

1. extend netconsole to multiple netpolls
   To send kernel messages to multiple logging agents, extend netcosnole
    to be able to use multiple netpolls. Each netpoll sends kernel messages
    to its own logging agent.

2. change config parameter format
   We change config parameter format from single configuration to multiple 
   configurations separated by ';'.

   ex) sending kernel messages to destination1 and destination2 using eth0.
    modprobe netconsole \
            netconsole="@/eth0,@[destination1]/;@/eth0,@[destination2]/"

3. introduce CONFIG_NETCONSOLE_DYNCON config to change between 
   existing netconsole and netconsole applying the above function.

Signed-off-by: Keiichi KII <k-keiichi@bx.jp.nec.com>
Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
---
Index: mm/drivers/net/netconsole.c
===================================================================
--- mm.orig/drivers/net/netconsole.c
+++ mm/drivers/net/netconsole.c
@@ -61,15 +61,102 @@ static struct netpoll np = {
 	.remote_port = 6666,
 	.remote_mac = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
 };
-static int configured = 0;
 
 #define MAX_PRINT_CHUNK 1000
 
+#ifdef CONFIG_NETCONSOLE_DYNCON
+struct netconsole_target {
+	struct list_head list;
+	int id;
+	struct netpoll np;
+};
+
+static LIST_HEAD(target_list);
+static DEFINE_SPINLOCK(target_list_lock);
+
+static int add_target(char* target_config);
+static void remove_target(struct netconsole_target *nt);
+static void cleanup_netconsole(void);
+
+static int add_target(char* target_config)
+{
+	int retval = 0;
+	static atomic_t target_count = ATOMIC_INIT(0);
+	struct netconsole_target *new_target;
+
+	new_target = kzalloc(sizeof(*new_target), GFP_KERNEL);
+	if (!new_target) {
+		printk(KERN_ERR "netconsole: kmalloc() failed!\n");
+		retval = -ENOMEM;
+		goto out;
+	}
+
+	new_target->np = np;
+	if (netpoll_parse_options(&new_target->np, target_config)) {
+		printk(KERN_ERR "netconsole: can't parse config:%s\n",
+		       target_config);
+		kfree(new_target);
+		retval = -EINVAL;
+		goto out;
+	}
+	if (netpoll_setup(&new_target->np)) {
+		printk(KERN_ERR "netconsole: can't setup netpoll:%s\n",
+		       target_config);
+		kfree(new_target);
+		retval = -EINVAL;
+		goto out;
+	}
+
+	new_target->id = atomic_inc_return(&target_count);
+
+	printk(KERN_INFO "netconsole: add target: "
+	       "remote ip_addr=%d.%d.%d.%d remote port=%d\n",
+	       HIPQUAD(new_target->np.remote_ip), new_target->np.remote_port);
+
+	spin_lock(&target_list_lock);
+	list_add(&new_target->list, &target_list);
+	spin_unlock(&target_list_lock);
+
+ out:
+	return retval;
+}
+
+static void remove_target(struct netconsole_target *nt)
+{
+	spin_lock(&target_list_lock);
+	list_del(&nt->list);
+	if (list_empty(&target_list))
+		netpoll_cleanup(&nt->np);
+	spin_unlock(&target_list_lock);
+	kfree(nt);
+}
+#endif /* CONFIG_NETCONSOLE_DYNCON */
+
 static void write_msg(struct console *con, const char *msg, unsigned int len)
 {
 	int frag, left;
 	unsigned long flags;
+#ifdef CONFIG_NETCONSOLE_DYNCON
+	struct netconsole_target *target;
+
+	if (list_empty(&target_list))
+		return;
 
+	local_irq_save(flags);
+	spin_lock(&target_list_lock);
+
+	for(left = len; left; ) {
+		frag = min(left, MAX_PRINT_CHUNK);
+		list_for_each_entry(target, &target_list, list) {
+			netpoll_send_udp(&target->np, msg, frag);
+		}
+		msg += frag;
+		left -= frag;
+	}
+
+	spin_unlock(&target_list_lock);
+	local_irq_restore(flags);
+#else
 	if (!np.dev)
 		return;
 
@@ -83,6 +170,7 @@ static void write_msg(struct console *co
 	}
 
 	local_irq_restore(flags);
+#endif /* CONFIG_NETCONSOLE_DYNCON */
 }
 
 static struct console netconsole = {
@@ -91,39 +179,60 @@ static struct console netconsole = {
 	.write = write_msg
 };
 
+#ifndef MODULE
 static int __init option_setup(char *opt)
 {
-	configured = !netpoll_parse_options(&np, opt);
+	strncpy(config, opt, 256);
 	return 1;
 }
 
 __setup("netconsole=", option_setup);
+#endif
 
 static int __init init_netconsole(void)
 {
-	int err;
+	char *tmp = config;
+#ifdef CONFIG_NETCONSOLE_DYNCON
+	char *p;
 
-	if(strlen(config))
-		option_setup(config);
-
-	if(!configured) {
-		printk("netconsole: not configured, aborting\n");
+	register_console(&netconsole);
+	if(!strlen(config)) {
+		printk(KERN_ERR "netconsole: not configured\n");
 		return 0;
 	}
-
-	err = netpoll_setup(&np);
-	if (err)
-		return err;
-
+	while ((p = strsep(&tmp, ";")) != NULL) {
+		if (add_target(p)) {
+			printk(KERN_ERR
+			       "netconsole: can't add target to netconsole\n");
+			cleanup_netconsole();
+			return -EINVAL;
+		}
+	}
+#else
+	if (!strlen(config) ||
+	    netpoll_parse_options(&np, tmp) || netpoll_setup(&np)) {
+		printk(KERN_ERR "netconsole: can't parse config:%s\n", tmp);
+		return -EINVAL;
+	}
 	register_console(&netconsole);
+#endif /* CONFIG_NETCONSOLE_DYNCON */
 	printk(KERN_INFO "netconsole: network logging started\n");
 	return 0;
 }
 
 static void cleanup_netconsole(void)
 {
+#ifdef CONFIG_NETCONSOLE_DYNCON
+	struct netconsole_target *nt, *tmp;
+
+	unregister_console(&netconsole);
+	list_for_each_entry_safe(nt, tmp, &target_list, list) {
+		remove_target(nt);
+	}
+#else
 	unregister_console(&netconsole);
 	netpoll_cleanup(&np);
+#endif /* CONFIG_NETCONSOLE_DYNCON */
 }
 
 module_init(init_netconsole);
Index: mm/drivers/net/Kconfig
===================================================================
--- mm.orig/drivers/net/Kconfig
+++ mm/drivers/net/Kconfig
@@ -2986,6 +2986,14 @@ config NETCONSOLE
 	If you want to log kernel messages over the network, enable this.
 	See <file:Documentation/networking/netconsole.txt> for details.
 
+config NETCONSOLE_DYNCON
+	default y
+	bool "Support for multiple logging and UI for netconsole"
+	depends on NETCONSOLE
+	---help---
+	This option enables multiple logging and changing dynamically
+	 configurations (e.g. IP adderss, port number and so on)
+	 by using sysfs and ioctl.
 endif #NETDEVICES
 
 config NETPOLL

-- 

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Stephen Hemminger]

On Wed, 18 Apr 2007 21:06:41 +0900
Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> From: Keiichi KII <k-keiichi@bx.jp.nec.com>
> 
> This patch contains the following changes for supporting multiple logging
>  agents.
> 
> 1. extend netconsole to multiple netpolls
>    To send kernel messages to multiple logging agents, extend netcosnole
>     to be able to use multiple netpolls. Each netpoll sends kernel messages
>     to its own logging agent.
> 
> 2. change config parameter format
>    We change config parameter format from single configuration to multiple 
>    configurations separated by ';'.
> 
>    ex) sending kernel messages to destination1 and destination2 using eth0.
>     modprobe netconsole \
>             netconsole="@/eth0,@[destination1]/;@/eth0,@[destination2]/"

Please include an update to Documentation/networking/netconsole.txt

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Keiichi KII]

> Please include an update to Documentation/networking/netconsole.txt

OK, I update the documnet for the added functions in next patches.

Thank you for your comments.
-- 
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: k-keiichi@bx.jp.nec.com

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Andrew Morton]

On Wed, 18 Apr 2007 21:06:41 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> From: Keiichi KII <k-keiichi@bx.jp.nec.com>
> 
> This patch contains the following changes for supporting multiple logging
>  agents.
> 
> 1. extend netconsole to multiple netpolls
>    To send kernel messages to multiple logging agents, extend netcosnole
>     to be able to use multiple netpolls. Each netpoll sends kernel messages
>     to its own logging agent.
> 
> 2. change config parameter format
>    We change config parameter format from single configuration to multiple 
>    configurations separated by ';'.
> 
>    ex) sending kernel messages to destination1 and destination2 using eth0.
>     modprobe netconsole \
>             netconsole="@/eth0,@[destination1]/;@/eth0,@[destination2]/"
> 
> 3. introduce CONFIG_NETCONSOLE_DYNCON config to change between 
>    existing netconsole and netconsole applying the above function.
> 
> Signed-off-by: Keiichi KII <k-keiichi@bx.jp.nec.com>
> Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
> ---
> Index: mm/drivers/net/netconsole.c
> ===================================================================
> --- mm.orig/drivers/net/netconsole.c
> +++ mm/drivers/net/netconsole.c
> @@ -61,15 +61,102 @@ static struct netpoll np = {
>  	.remote_port = 6666,
>  	.remote_mac = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
>  };
> -static int configured = 0;
>  
>  #define MAX_PRINT_CHUNK 1000
>  
> +#ifdef CONFIG_NETCONSOLE_DYNCON
> +struct netconsole_target {
> +	struct list_head list;
> +	int id;
> +	struct netpoll np;
> +};
> +
> +static LIST_HEAD(target_list);
> +static DEFINE_SPINLOCK(target_list_lock);
> +
> +static int add_target(char* target_config);
> +static void remove_target(struct netconsole_target *nt);
> +static void cleanup_netconsole(void);

I started to do some cleanups and fixups here, but abandoned it when it was
all getting a bit large.

Here are some fixes against this patch:


- Fix Kconfig

- Avoid need for forward-declared statics

- Coding style:

	fix `if' layout
	unneeded braces

- use spin_lock_irqsave() and _restore()



diff -puN drivers/net/Kconfig~netconsole-support-multiple-logging-fix drivers/net/Kconfig
--- a/drivers/net/Kconfig~netconsole-support-multiple-logging-fix
+++ a/drivers/net/Kconfig
@@ -2965,9 +2965,11 @@ config NETCONSOLE_DYNCON
 	bool "Support for multiple logging and UI for netconsole"
 	depends on NETCONSOLE
 	---help---
-	This option enables multiple logging and changing dynamically
-	 configurations (e.g. IP adderss, port number and so on)
-	 by using sysfs and ioctl.
+	  This option enables multiple logging and changing dynamically
+	  configurations (e.g. IP address, port number and so on)
+	  by using sysfs and ioctl.
+	  See <file:Documentation/networking/netconsole.txt> for details.
+
 endif #NETDEVICES
 
 config NETPOLL
diff -puN drivers/net/netconsole.c~netconsole-support-multiple-logging-fix drivers/net/netconsole.c
--- a/drivers/net/netconsole.c~netconsole-support-multiple-logging-fix
+++ a/drivers/net/netconsole.c
@@ -74,10 +74,6 @@ struct netconsole_target {
 static LIST_HEAD(target_list);
 static DEFINE_SPINLOCK(target_list_lock);
 
-static int add_target(char* target_config);
-static void remove_target(struct netconsole_target *nt);
-static void cleanup_netconsole(void);
-
 static int add_target(char* target_config)
 {
 	int retval = 0;
@@ -142,27 +138,24 @@ static void write_msg(struct console *co
 	if (list_empty(&target_list))
 		return;
 
-	local_irq_save(flags);
-	spin_lock(&target_list_lock);
+	spin_lock_irqsave(&target_list_lock, flags);
 
-	for(left = len; left; ) {
+	for (left = len; left; ) {
 		frag = min(left, MAX_PRINT_CHUNK);
-		list_for_each_entry(target, &target_list, list) {
+		list_for_each_entry(target, &target_list, list)
 			netpoll_send_udp(&target->np, msg, frag);
-		}
 		msg += frag;
 		left -= frag;
 	}
 
-	spin_unlock(&target_list_lock);
-	local_irq_restore(flags);
+	spin_unlock_irqrestore(&target_list_lock, flags);
 #else
 	if (!np.dev)
 		return;
 
 	local_irq_save(flags);
 
-	for(left = len; left; ) {
+	for (left = len; left; ) {
 		frag = min(left, MAX_PRINT_CHUNK);
 		netpoll_send_udp(&np, msg, frag);
 		msg += frag;
@@ -189,6 +182,20 @@ static int __init option_setup(char *opt
 __setup("netconsole=", option_setup);
 #endif
 
+static void cleanup_netconsole(void)
+{
+#ifdef CONFIG_NETCONSOLE_DYNCON
+	struct netconsole_target *nt, *tmp;
+
+	unregister_console(&netconsole);
+	list_for_each_entry_safe(nt, tmp, &target_list, list)
+		remove_target(nt);
+#else
+	unregister_console(&netconsole);
+	netpoll_cleanup(&np);
+#endif /* CONFIG_NETCONSOLE_DYNCON */
+}
+
 static int __init init_netconsole(void)
 {
 	char *tmp = config;
@@ -196,7 +203,7 @@ static int __init init_netconsole(void)
 	char *p;
 
 	register_console(&netconsole);
-	if(!strlen(config)) {
+	if (!strlen(config)) {
 		printk(KERN_ERR "netconsole: not configured\n");
 		return 0;
 	}
@@ -220,20 +227,5 @@ static int __init init_netconsole(void)
 	return 0;
 }
 
-static void cleanup_netconsole(void)
-{
-#ifdef CONFIG_NETCONSOLE_DYNCON
-	struct netconsole_target *nt, *tmp;
-
-	unregister_console(&netconsole);
-	list_for_each_entry_safe(nt, tmp, &target_list, list) {
-		remove_target(nt);
-	}
-#else
-	unregister_console(&netconsole);
-	netpoll_cleanup(&np);
-#endif /* CONFIG_NETCONSOLE_DYNCON */
-}
-
 module_init(init_netconsole);
 module_exit(cleanup_netconsole);
_

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Keiichi KII]

> I started to do some cleanups and fixups here, but abandoned it when it was
> all getting a bit large.
> 
> Here are some fixes against this patch:

I'm going to fix my patches by following your reviews and send new patches 
on the LKML and the netdev ML in a few days.

Thank you for your comments and reviews.
-- 
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: k-keiichi@bx.jp.nec.com

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Andrew Morton]

On Fri, 20 Apr 2007 18:51:13 +0900
Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> > I started to do some cleanups and fixups here, but abandoned it when it was
> > all getting a bit large.
> > 
> > Here are some fixes against this patch:
> 
> I'm going to fix my patches by following your reviews and send new patches 
> on the LKML and the netdev ML in a few days.
> 

Well..  before you can finish this work we need to decide upon what the
interface to userspace will be.

- The miscdev isn't appropriate

- netlink remains a possibility

- Stephen suggests an ioctl against a socket and davem suggests socket
  options, but it's unclear to me how that socket will get bound to
  netconsole?


either way, I agree with the overall thrust of this work: netconsole is
useful in production environments, can become more useful and will need
runtime configurability.


I wonder if we're approaching this in the right way, however...

At a high level, netconsole is just a flow of UDP packets between two
machines.  The kernel already has rich and well-understood ways of creating
and configuring such flows.

So...  instead of creating a brand new way of configuring such a flow via
sysfs and ioctl, could we instead create a flow using the existing
mechanisms (presumably the socket API) and then "transfer" the information
from that flow over to netconsole by some means??

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Matt Mackall]

On Fri, Apr 20, 2007 at 11:15:26AM -0700, Andrew Morton wrote:
> On Fri, 20 Apr 2007 18:51:13 +0900
> Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> 
> > > I started to do some cleanups and fixups here, but abandoned it when it was
> > > all getting a bit large.
> > > 
> > > Here are some fixes against this patch:
> > 
> > I'm going to fix my patches by following your reviews and send new patches 
> > on the LKML and the netdev ML in a few days.
> > 
> 
> Well..  before you can finish this work we need to decide upon what the
> interface to userspace will be.
> 
> - The miscdev isn't appropriate
> 
> - netlink remains a possibility
> 
> - Stephen suggests an ioctl against a socket and davem suggests socket
>   options, but it's unclear to me how that socket will get bound to
>   netconsole?

Yeah, that's a bit of a head-scratcher.

> either way, I agree with the overall thrust of this work: netconsole is
> useful in production environments, can become more useful and will need
> runtime configurability.
> 
> 
> I wonder if we're approaching this in the right way, however...
> 
> At a high level, netconsole is just a flow of UDP packets between two
> machines.  The kernel already has rich and well-understood ways of creating
> and configuring such flows.
> 
> So...  instead of creating a brand new way of configuring such a flow via
> sysfs and ioctl, could we instead create a flow using the existing
> mechanisms (presumably the socket API) and then "transfer" the information
> from that flow over to netconsole by some means??

We don't really have anything that corresponds to netpoll's
connections at higher levels.

I'm tempted to say we should make this work more like the dummy
network device. ie:

modprobe netconsole -o netcon1 [params]
modprobe netconsole -o netcon2 [params]

-- 
Mathematics is the supreme nostalgia of our time.

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Stephen Hemminger]

On Fri, 20 Apr 2007 13:21:10 -0500
Matt Mackall <mpm@selenic.com> wrote:

> On Fri, Apr 20, 2007 at 11:15:26AM -0700, Andrew Morton wrote:
> > On Fri, 20 Apr 2007 18:51:13 +0900
> > Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> > 
> > > > I started to do some cleanups and fixups here, but abandoned it when it was
> > > > all getting a bit large.
> > > > 
> > > > Here are some fixes against this patch:
> > > 
> > > I'm going to fix my patches by following your reviews and send new patches 
> > > on the LKML and the netdev ML in a few days.
> > > 
> > 
> > Well..  before you can finish this work we need to decide upon what the
> > interface to userspace will be.
> > 
> > - The miscdev isn't appropriate
> > 
> > - netlink remains a possibility
> > 
> > - Stephen suggests an ioctl against a socket and davem suggests socket
> >   options, but it's unclear to me how that socket will get bound to
> >   netconsole?
> 
> Yeah, that's a bit of a head-scratcher.
> 
> > either way, I agree with the overall thrust of this work: netconsole is
> > useful in production environments, can become more useful and will need
> > runtime configurability.
> > 
> > 
> > I wonder if we're approaching this in the right way, however...
> > 
> > At a high level, netconsole is just a flow of UDP packets between two
> > machines.  The kernel already has rich and well-understood ways of creating
> > and configuring such flows.
> > 
> > So...  instead of creating a brand new way of configuring such a flow via
> > sysfs and ioctl, could we instead create a flow using the existing
> > mechanisms (presumably the socket API) and then "transfer" the information
> > from that flow over to netconsole by some means??
> 
> We don't really have anything that corresponds to netpoll's
> connections at higher levels.
> 
> I'm tempted to say we should make this work more like the dummy
> network device. ie:
> 
> modprobe netconsole -o netcon1 [params]
> modprobe netconsole -o netcon2 [params]

The configuration of netconsole's looks like the configuration of routes.
Granted you probably have more routes than netconsoles, but the interface
issues are similar.  Netlink with a small application wouldn't be nice.
And having /proc/net/netconsole (read-only) would be good for the netlink
impaired.

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Keiichi KII]

>> We don't really have anything that corresponds to netpoll's
>> connections at higher levels.
>>
>> I'm tempted to say we should make this work more like the dummy
>> network device. ie:
>>
>> modprobe netconsole -o netcon1 [params]
>> modprobe netconsole -o netcon2 [params]
> 
> The configuration of netconsole's looks like the configuration of routes.
> Granted you probably have more routes than netconsoles, but the interface
> issues are similar.  Netlink with a small application wouldn't be nice.
> And having /proc/net/netconsole (read-only) would be good for the netlink
> impaired.

Do you say that we had better use procfs instead of sysfs to show the 
configurations of netconsole?

If so, I have a question.
I thought that "procfs use things related to process as far as possible".
Is it no problem to use procfs here? 

-- 
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: k-keiichi@bx.jp.nec.com

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Keiichi KII]

> On Fri, 20 Apr 2007 18:51:13 +0900
> Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> 
>>> I started to do some cleanups and fixups here, but abandoned it when it was
>>> all getting a bit large.
>>>
>>> Here are some fixes against this patch:
>> I'm going to fix my patches by following your reviews and send new patches 
>> on the LKML and the netdev ML in a few days.
>>
> 
> Well..  before you can finish this work we need to decide upon what the
> interface to userspace will be.
> 
> - The miscdev isn't appropriate
> 

Why isn't miscdev appropriate? 
We just shouldn't use miscdev for networking conventionally?

-- 
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: k-keiichi@bx.jp.nec.com

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Andrew Morton]

On Tue, 24 Apr 2007 17:14:28 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> > On Fri, 20 Apr 2007 18:51:13 +0900
> > Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> > 
> >>> I started to do some cleanups and fixups here, but abandoned it when it was
> >>> all getting a bit large.
> >>>
> >>> Here are some fixes against this patch:
> >> I'm going to fix my patches by following your reviews and send new patches 
> >> on the LKML and the netdev ML in a few days.
> >>
> > 
> > Well..  before you can finish this work we need to decide upon what the
> > interface to userspace will be.
> > 
> > - The miscdev isn't appropriate
> > 
> 
> Why isn't miscdev appropriate? 
> We just shouldn't use miscdev for networking conventionally?
> 

Yes it's rather odd, especially for networking.

What does the miscdev _do_ anyway?  Is it purely a target for the ioctls?

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Stephen Hemminger]

On Tue, 24 Apr 2007 01:29:02 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 24 Apr 2007 17:14:28 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> 
> > > On Fri, 20 Apr 2007 18:51:13 +0900
> > > Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> > > 
> > >>> I started to do some cleanups and fixups here, but abandoned it when it was
> > >>> all getting a bit large.
> > >>>
> > >>> Here are some fixes against this patch:
> > >> I'm going to fix my patches by following your reviews and send new patches 
> > >> on the LKML and the netdev ML in a few days.
> > >>
> > > 
> > > Well..  before you can finish this work we need to decide upon what the
> > > interface to userspace will be.
> > > 
> > > - The miscdev isn't appropriate
> > > 
> > 
> > Why isn't miscdev appropriate? 
> > We just shouldn't use miscdev for networking conventionally?
> > 
> 
> Yes it's rather odd, especially for networking.
> 
> What does the miscdev _do_ anyway?  Is it purely a target for the ioctls?

Some other speculations:
1. Would it be possible to add ioctl's to /dev/console? This would be more in
    keeping with older Unix style model.

2. Using sysfs makes sense if there is a device object that exists to
   add the sysfs attributes to.

3. Procfs is handy for summary type tables.

4. Netlink does feel like overkill for this. Although newer generic netlink
   makes it easier.

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[Keiichi KII]

>>>> Well..  before you can finish this work we need to decide upon what the
>>>> interface to userspace will be.
>>>>
>>>> - The miscdev isn't appropriate
>>>>
>>> Why isn't miscdev appropriate? 
>>> We just shouldn't use miscdev for networking conventionally?
>>>
>> Yes it's rather odd, especially for networking.
>>
>> What does the miscdev _do_ anyway?  Is it purely a target for the ioctls?

Yes, I purely use miscdev for the ioctls.

I want to use sysfs and ioctl to implement the dynamic configurabillity.
The sysfs shows/changes netconsole configurations(IP address, port and so on).
A userland application using the ioctl adds/removes netconsole port.

I thought that the dynamic configurability could be realized without a 
userland application. in the kernel only.
(e.g. only sysfs, no userland application)
But I think we need the function to automatically resolve the destination 
MAC address from IP address because of the resolving cost and 
I should implement a userland application, not netconsole kernel module.
The netconsle will become more useful by implementing the above function.

> Some other speculations:
> 1. Would it be possible to add ioctl's to /dev/console? This would be more in
>     keeping with older Unix style model.
> 
> 2. Using sysfs makes sense if there is a device object that exists to
>    add the sysfs attributes to.
> 
> 3. Procfs is handy for summary type tables.
> 
> 4. Netlink does feel like overkill for this. Although newer generic netlink
>    makes it easier.

If I use sysfs, Is it proper location that adds each attributes of netconsole 
port in "/sys/class/misc/netconsole/port[0-9]*", or another locations in /sys/?

Stephen Hemminger said "The configuration of netconsole's looks like the 
configuration of routes".
I think so too.
So I think ioctl commands for adding/removing port and the following userland 
application like route(8) command by using the ioctl.

e.g.
1. add port
# netconfig add 192.168.0.10 6666

2. remove port
# netconfig remove 1

3. show port info
# netconfig
id status  Source IP   Source Port Destination IP Destination Port Destination MAC
1  enable  192.168.0.1 6665        192.168.0.10   6666             00:11:22:33:44:55
2  disable 192.168.0.1 6665        192.168.0.20   6666             00:11:22:33:44:66

route(8) command uses ioctl for Netlink.
But, I'm going to implement ioctl's to /dev/console because of the above comments.

Thank you for your comments.
Any comments very welcome.
-- 
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: k-keiichi@bx.jp.nec.com

Re: [RFC][PATCH -mm take4 2/6] support multiple logging

[David Miller]

From: Keiichi KII <k-keiichi@bx.jp.nec.com>
Date: Thu, 26 Apr 2007 13:02:04 +0900

> Stephen Hemminger said "The configuration of netconsole's looks like the 
> configuration of routes".
> I think so too.
> So I think ioctl commands for adding/removing port and the following userland 
> application like route(8) command by using the ioctl.

Like the route command itself, the route changing ioctl()s are
old deprecated BSD compatible functionality.

All current routing configuration is done using netlink and the 'ip'
utility.

[RFC][PATCH -mm take4 3/6] add interface for netconsole using sysfs

[Keiichi KII]

From: Keiichi KII <k-keiichi@bx.jp.nec.com>

This patch contains the following changes.

create a sysfs entry for netconsole in /sys/class/misc.
This entry has elements related to netconsole as follows.
You can change configuration of netconsole(writable attributes such as IP
address, port number and so on) and check current configuration of netconsole.

-+- /sys/class/misc/
 |-+- netconsole/
   |-+- port1/
   | |--- id          [r--r--r--]  unique port id
   | |--- local_ip    [rw-r--r--]  source IP to use, writable
   | |--- local_mac   [r--r--r--]  source MAC address
   | |--- local_port  [rw-r--r--]  source port number for UDP packets, writable
   | |--- remote_ip   [rw-r--r--]  port number for logging agent, writable
   | |--- remote_mac  [rw-r--r--]  MAC address for logging agent, writable
   | ---- remote_port [rw-r--r--]  IP address for logging agent, writable
   |--- port2/
   |--- port3/
   ...

Signed-off-by: Keiichi KII <k-keiichi@bx.jp.nec.com>
Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
---
Index: mm/drivers/net/netconsole.c
===================================================================
--- mm.orig/drivers/net/netconsole.c
+++ mm/drivers/net/netconsole.c
@@ -45,6 +45,8 @@
 #include <linux/sysrq.h>
 #include <linux/smp.h>
 #include <linux/netpoll.h>
+#include <linux/miscdevice.h>
+#include <linux/inet.h>
 
 MODULE_AUTHOR("Maintainer: Matt Mackall <mpm@selenic.com>");
 MODULE_DESCRIPTION("Console driver for network interfaces");
@@ -67,6 +69,7 @@ static struct netpoll np = {
 #ifdef CONFIG_NETCONSOLE_DYNCON
 struct netconsole_target {
 	struct list_head list;
+	struct kobject obj;
 	int id;
 	struct netpoll np;
 };
@@ -77,6 +80,207 @@ static DEFINE_SPINLOCK(target_list_lock)
 static int add_target(char* target_config);
 static void remove_target(struct netconsole_target *nt);
 static void cleanup_netconsole(void);
+static int setup_target_sysfs(struct netconsole_target *nt);
+
+static int miscdev_configured;
+
+static ssize_t show_id(struct netconsole_target *nt, char *buf)
+{
+	return sprintf(buf, "%d\n", nt->id);
+}
+
+static ssize_t show_local_port(struct netconsole_target *nt, char *buf)
+{
+	return sprintf(buf, "%d\n", nt->np.local_port);
+}
+
+static ssize_t show_remote_port(struct netconsole_target *nt, char *buf)
+{
+	return sprintf(buf, "%d\n", nt->np.remote_port);
+}
+
+static ssize_t show_local_ip(struct netconsole_target *nt, char *buf)
+{
+	return sprintf(buf, "%d.%d.%d.%d\n", HIPQUAD(nt->np.local_ip));
+}
+
+static ssize_t show_remote_ip(struct netconsole_target *nt, char *buf)
+{
+	return sprintf(buf, "%d.%d.%d.%d\n", HIPQUAD(nt->np.remote_ip));
+}
+
+static ssize_t show_local_mac(struct netconsole_target *nt, char *buf)
+{
+	return sprintf(buf, "%02x:%02x:%02x:%02x:%02x:%02x\n",
+		       nt->np.local_mac[0], nt->np.local_mac[1],
+		       nt->np.local_mac[2], nt->np.local_mac[3],
+		       nt->np.local_mac[4], nt->np.local_mac[5]);
+}
+
+static ssize_t show_remote_mac(struct netconsole_target *nt, char *buf)
+{
+	return sprintf(buf, "%02x:%02x:%02x:%02x:%02x:%02x\n",
+		       nt->np.remote_mac[0], nt->np.remote_mac[1],
+		       nt->np.remote_mac[2], nt->np.remote_mac[3],
+		       nt->np.remote_mac[4], nt->np.remote_mac[5]);
+}
+
+static ssize_t store_local_port(struct netconsole_target *nt, const char *buf,
+				size_t count)
+{
+	spin_lock(&target_list_lock);
+	nt->np.local_port = simple_strtol(buf, NULL, 10);
+	spin_unlock(&target_list_lock);
+
+	return count;
+}
+
+static ssize_t store_remote_port(struct netconsole_target *nt, const char *buf,
+				size_t count)
+{
+	spin_lock(&target_list_lock);
+	nt->np.remote_port = simple_strtol(buf, NULL, 10);
+	spin_unlock(&target_list_lock);
+
+	return count;
+}
+
+static ssize_t store_local_ip(struct netconsole_target *nt, const char *buf,
+			      size_t count)
+{
+	spin_lock(&target_list_lock);
+	nt->np.local_ip = ntohl(in_aton(buf));
+	spin_unlock(&target_list_lock);
+
+	return count;
+}
+
+static ssize_t store_remote_ip(struct netconsole_target *nt, const char *buf,
+			       size_t count)
+{
+	spin_lock(&target_list_lock);
+	nt->np.remote_ip = ntohl(in_aton(buf));
+	spin_unlock(&target_list_lock);
+
+	return count;
+}
+
+static ssize_t store_remote_mac(struct netconsole_target *nt, const char *buf,
+			       size_t count)
+{
+	unsigned char input_mac[ETH_ALEN] =
+		{0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+	const char *cur = buf;
+	int i = 0;
+
+	input_mac[i++] = simple_strtol(cur, NULL, 16);
+	while ((cur = strchr(cur, ':')) != NULL) {
+		cur++;
+		input_mac[i++] = simple_strtol(cur, NULL, 16);
+	}
+	if (i != ETH_ALEN)
+		return -EINVAL;
+	spin_lock(&target_list_lock);
+	memcpy(nt->np.remote_mac, input_mac, ETH_ALEN);
+	spin_unlock(&target_list_lock);
+
+	return count;
+}
+
+struct target_attr {
+	struct attribute attr;
+	ssize_t (*show)(struct netconsole_target*, char*);
+	ssize_t (*store)(struct netconsole_target*, const char*,
+			 size_t count);
+};
+
+#define NETCON_CLASS_ATTR(_name, _mode, _show, _store) \
+struct target_attr target_attr_##_name = \
+                           __ATTR(_name, _mode, _show, _store)
+
+static NETCON_CLASS_ATTR(id, S_IRUGO, show_id, NULL);
+static NETCON_CLASS_ATTR(local_port, S_IRUGO | S_IWUSR,
+			 show_local_port, store_local_port);
+static NETCON_CLASS_ATTR(remote_port, S_IRUGO | S_IWUSR,
+			 show_remote_port, store_remote_port);
+static NETCON_CLASS_ATTR(local_ip, S_IRUGO | S_IWUSR,
+			 show_local_ip, store_local_ip);
+static NETCON_CLASS_ATTR(remote_ip, S_IRUGO | S_IWUSR,
+			 show_remote_ip, store_remote_ip);
+static NETCON_CLASS_ATTR(local_mac, S_IRUGO, show_local_mac, NULL);
+static NETCON_CLASS_ATTR(remote_mac, S_IRUGO | S_IWUSR,
+			 show_remote_mac, store_remote_mac);
+
+static struct attribute *target_attrs[] = {
+	&target_attr_id.attr,
+	&target_attr_local_port.attr,
+	&target_attr_remote_port.attr,
+	&target_attr_local_ip.attr,
+	&target_attr_remote_ip.attr,
+	&target_attr_local_mac.attr,
+	&target_attr_remote_mac.attr,
+	NULL
+};
+
+static ssize_t show_target_attr(struct kobject *kobj,
+				    struct attribute *attr,
+				    char *buffer)
+{
+	struct target_attr *na =
+		container_of(attr, struct target_attr, attr);
+	struct netconsole_target * nt =
+		container_of(kobj, struct netconsole_target, obj);
+	if (na->show)
+		return na->show(nt, buffer);
+	else
+		return -EIO;
+}
+
+static ssize_t store_target_attr(struct kobject *kobj,
+				     struct attribute *attr,
+				     const char *buffer, size_t count)
+{
+	struct target_attr *na =
+		container_of(attr, struct target_attr, attr);
+	struct netconsole_target *nt =
+		container_of(kobj, struct netconsole_target, obj);
+	if (na->store)
+		return na->store(nt, buffer, count);
+	else
+		return -EIO;
+}
+
+static void release_target(struct kobject *kobj)
+{
+	struct netconsole_target *nt =
+		container_of(kobj, struct netconsole_target, obj);
+
+	remove_target(nt);
+}
+
+static struct sysfs_ops target_sysfs_ops = {
+	.show = show_target_attr,
+	.store = store_target_attr
+};
+
+static struct kobj_type target_ktype = {
+	.release = release_target,
+	.sysfs_ops = &target_sysfs_ops,
+	.default_attrs = target_attrs,
+};
+
+static struct miscdevice netconsole_miscdev = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "netconsole",
+};
+
+static int setup_target_sysfs(struct netconsole_target *nt)
+{
+	kobject_set_name(&nt->obj, "port%d", nt->id);
+	nt->obj.parent = &netconsole_miscdev.this_device->kobj;
+	nt->obj.ktype = &target_ktype;
+	return kobject_register(&nt->obj);
+}
 
 static int add_target(char* target_config)
 {
@@ -117,6 +321,7 @@ static int add_target(char* target_confi
 	list_add(&new_target->list, &target_list);
 	spin_unlock(&target_list_lock);
 
+	retval = setup_target_sysfs(new_target);
  out:
 	return retval;
 }
@@ -195,6 +400,15 @@ static int __init init_netconsole(void)
 #ifdef CONFIG_NETCONSOLE_DYNCON
 	char *p;
 
+	if (misc_register(&netconsole_miscdev)) {
+		printk(KERN_ERR
+		       "netconsole: failed to register misc device for "
+		       "name = %s, id = %d\n",
+		       netconsole_miscdev.name, netconsole_miscdev.minor);
+		return -EIO;
+	} else
+		miscdev_configured = 1;
+
 	register_console(&netconsole);
 	if(!strlen(config)) {
 		printk(KERN_ERR "netconsole: not configured\n");
@@ -227,8 +441,10 @@ static void cleanup_netconsole(void)
 
 	unregister_console(&netconsole);
 	list_for_each_entry_safe(nt, tmp, &target_list, list) {
-		remove_target(nt);
+		kobject_unregister(&nt->obj);
 	}
+	if (miscdev_configured)
+		misc_deregister(&netconsole_miscdev);
 #else
 	unregister_console(&netconsole);
 	netpoll_cleanup(&np);

-- 

Re: [RFC][PATCH -mm take4 3/6] add interface for netconsole using sysfs

[Andrew Morton]

On Wed, 18 Apr 2007 21:08:45 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> +static ssize_t store_local_port(struct netconsole_target *nt, const char *buf,
> +				size_t count)
> +{
> +	spin_lock(&target_list_lock);
> +	nt->np.local_port = simple_strtol(buf, NULL, 10);
> +	spin_unlock(&target_list_lock);
> +
> +	return count;
> +}
> +
> +static ssize_t store_remote_port(struct netconsole_target *nt, const char *buf,
> +				size_t count)
> +{
> +	spin_lock(&target_list_lock);
> +	nt->np.remote_port = simple_strtol(buf, NULL, 10);
> +	spin_unlock(&target_list_lock);
> +
> +	return count;
> +}

I think that you'll find that the locking in here does nothing useful and
can be removed.


Also, write_msg() can be called from IRQ context, so this lock _must_ be
taken with spin_lock_irq[save] basically everywhere - the code as-is can be
deadlocked.

[RFC][PATCH -mm take4 4/6] using symlink for the net_device

[Keiichi KII]

From: Keiichi KII <k-keiichi@bx.jp.nec.com>

We use symbolic link for net_device.
The link in sysfs represents the corresponding network etherdevice.

-+- /sys/class/misc/
 |-+- netconsole/
 |-+- port1/
 | |--- id             [r--r--r--]  id
 | |--- net:<net_dev>  [rw-r--r--]  net_dev: eth0,eth1,...
 | ...
 |--- port2/
 ...

Signed-off-by: Keiichi KII <k-keiichi@bx.jp.nec.com>
Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
---
Index: mm/drivers/net/netconsole.c
===================================================================
--- mm.orig/drivers/net/netconsole.c
+++ mm/drivers/net/netconsole.c
@@ -81,6 +81,9 @@ static int add_target(char* target_confi
 static void remove_target(struct netconsole_target *nt);
 static void cleanup_netconsole(void);
 static int setup_target_sysfs(struct netconsole_target *nt);
+static char *make_netdev_class_name(char *netdev_name);
+static int netconsole_event(struct notifier_block *this, unsigned long event,
+			    void *ptr);
 
 static int miscdev_configured;
 
@@ -274,12 +277,77 @@ static struct miscdevice netconsole_misc
 	.name = "netconsole",
 };
 
+static struct notifier_block netconsole_notifier = {
+	.notifier_call = netconsole_event,
+};
+
 static int setup_target_sysfs(struct netconsole_target *nt)
 {
+	int retval = 0;
+	char *name;
+
 	kobject_set_name(&nt->obj, "port%d", nt->id);
 	nt->obj.parent = &netconsole_miscdev.this_device->kobj;
 	nt->obj.ktype = &target_ktype;
-	return kobject_register(&nt->obj);
+	retval = kobject_register(&nt->obj);
+	name = make_netdev_class_name(nt->np.dev_name);
+	if (IS_ERR(name))
+		return PTR_ERR(name);
+	retval = sysfs_create_link(&nt->obj, &nt->np.dev->dev.kobj, name);
+	kfree(name);
+
+	return retval;
+}
+
+static char *make_netdev_class_name(char *netdev_name)
+{
+	int size;
+	char *name;
+	char *netdev_class_prefix = "net:";
+
+	size = strlen(netdev_class_prefix) + strlen(netdev_name) + 1;
+	name = kmalloc(size, GFP_KERNEL);
+	if (!name) {
+		printk(KERN_ERR "netconsole: kmalloc() failed!\n");
+		return ERR_PTR(-ENOMEM);
+	}
+	strcpy(name, netdev_class_prefix);
+	strcat(name, netdev_name);
+
+	return name;
+}
+
+static int netconsole_event(struct notifier_block *this, unsigned long event,
+			    void *ptr)
+{
+	int error = 0;
+	char *old_link_name = NULL, *new_link_name = NULL;
+	struct netconsole_target *nt;
+	struct net_device *dev = ptr;
+
+	if (event == NETDEV_CHANGENAME) {
+		spin_lock(&target_list_lock);
+		list_for_each_entry(nt, &target_list, list) {
+			if (nt->np.dev != dev)
+				continue;
+			new_link_name = make_netdev_class_name(dev->name);
+			old_link_name =
+				make_netdev_class_name(nt->np.dev_name);
+			sysfs_remove_link(&nt->obj, old_link_name);
+			error = sysfs_create_link(&nt->obj,
+						  &nt->np.dev->dev.kobj,
+						  new_link_name);
+			if (error)
+				printk(KERN_ERR "can't create link: %s\n",
+				       new_link_name);
+			strcpy(nt->np.dev_name, dev->name);
+			kfree(new_link_name);
+			kfree(old_link_name);
+		}
+		spin_unlock(&target_list_lock);
+	}
+
+	return NOTIFY_DONE;
 }
 
 static int add_target(char* target_config)
@@ -409,6 +477,7 @@ static int __init init_netconsole(void)
 	} else
 		miscdev_configured = 1;
 
+	register_netdevice_notifier(&netconsole_notifier);
 	register_console(&netconsole);
 	if(!strlen(config)) {
 		printk(KERN_ERR "netconsole: not configured\n");
@@ -443,6 +512,7 @@ static void cleanup_netconsole(void)
 	list_for_each_entry_safe(nt, tmp, &target_list, list) {
 		kobject_unregister(&nt->obj);
 	}
+	unregister_netdevice_notifier(&netconsole_notifier);
 	if (miscdev_configured)
 		misc_deregister(&netconsole_miscdev);
 #else

-- 

Re: [RFC][PATCH -mm take4 4/6] using symlink for the net_device

[Andrew Morton]

On Wed, 18 Apr 2007 21:11:14 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> From: Keiichi KII <k-keiichi@bx.jp.nec.com>
> 
> We use symbolic link for net_device.

As Stephen said, please fully document the new interfaces in netconsole.txt.

Please also cc netdev@vger.kernel.org on all networking-related patches.

> +static char *make_netdev_class_name(char *netdev_name);
> +static int netconsole_event(struct notifier_block *this, unsigned long event,
> +			    void *ptr);

Please try order things in a way which minimises the number of
forward-declarations, as long as such ordering doesn't make the code
illogical (it usually doesn't).

>  static int miscdev_configured;
>  
> @@ -274,12 +277,77 @@ static struct miscdevice netconsole_misc
>  	.name = "netconsole",
>  };
>  
> +static struct notifier_block netconsole_notifier = {
> +	.notifier_call = netconsole_event,
> +};
> +
>  static int setup_target_sysfs(struct netconsole_target *nt)
>  {
> +	int retval = 0;
> +	char *name;
> +
>  	kobject_set_name(&nt->obj, "port%d", nt->id);
>  	nt->obj.parent = &netconsole_miscdev.this_device->kobj;
>  	nt->obj.ktype = &target_ktype;
> -	return kobject_register(&nt->obj);
> +	retval = kobject_register(&nt->obj);
> +	name = make_netdev_class_name(nt->np.dev_name);
> +	if (IS_ERR(name))
> +		return PTR_ERR(name);
> +	retval = sysfs_create_link(&nt->obj, &nt->np.dev->dev.kobj, name);
> +	kfree(name);
> +
> +	return retval;
> +}
> +
> +static char *make_netdev_class_name(char *netdev_name)
> +{
> +	int size;
> +	char *name;
> +	char *netdev_class_prefix = "net:";
> +
> +	size = strlen(netdev_class_prefix) + strlen(netdev_name) + 1;
> +	name = kmalloc(size, GFP_KERNEL);
> +	if (!name) {
> +		printk(KERN_ERR "netconsole: kmalloc() failed!\n");
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	strcpy(name, netdev_class_prefix);
> +	strcat(name, netdev_name);
> +
> +	return name;
> +}

I think this whole function can be replaced by one call to kasprintf()

> +static int netconsole_event(struct notifier_block *this, unsigned long event,
> +			    void *ptr)
> +{
> +	int error = 0;
> +	char *old_link_name = NULL, *new_link_name = NULL;
> +	struct netconsole_target *nt;
> +	struct net_device *dev = ptr;
> +
> +	if (event == NETDEV_CHANGENAME) {
> +		spin_lock(&target_list_lock);
> +		list_for_each_entry(nt, &target_list, list) {
> +			if (nt->np.dev != dev)
> +				continue;
> +			new_link_name = make_netdev_class_name(dev->name);
> +			old_link_name =
> +				make_netdev_class_name(nt->np.dev_name);

The error return from make_netdev_class_name() is being ignored here.

> +			sysfs_remove_link(&nt->obj, old_link_name);
> +			error = sysfs_create_link(&nt->obj,
> +						  &nt->np.dev->dev.kobj,
> +						  new_link_name);
> +			if (error)
> +				printk(KERN_ERR "can't create link: %s\n",
> +				       new_link_name);
> +			strcpy(nt->np.dev_name, dev->name);
> +			kfree(new_link_name);
> +			kfree(old_link_name);
> +		}
> +		spin_unlock(&target_list_lock);
> +	}
> +
> +	return NOTIFY_DONE;
>  }

[RFC][PATCH -mm take4 5/6] switch function of netpoll

[Keiichi KII]

From: Keiichi KII <k-keiichi@bx.jp.nec.com>

This patch contains switch function of netpoll.

If "enabled" attribute of certain port is '1', this port is used
and the configurations of this port are uable to change.

If "enabled" attribute of certain port is '0', this port isn't used
and the configurations of this port are able to change.

-+- /sys/class/misc/
|-+- netconsole/
  |-+- port1/
  | |--- id          [r--r--r--]  id
  | |--- enabled     [rw-r--r--]  0: disable 1: enable, writable
  | ...
  |--- port2/
  ...

Signed-off-by: Keiichi KII <k-keiichi@bx.jp.nec.com>
Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
---
Index: mm/drivers/net/netconsole.c
===================================================================
--- mm.orig/drivers/net/netconsole.c
+++ mm/drivers/net/netconsole.c
@@ -71,6 +71,7 @@ struct netconsole_target {
 	struct list_head list;
 	struct kobject obj;
 	int id;
+	int enabled;
 	struct netpoll np;
 };
 
@@ -128,10 +129,19 @@ static ssize_t show_remote_mac(struct ne
 		       nt->np.remote_mac[4], nt->np.remote_mac[5]);
 }
 
+static ssize_t show_enabled(struct netconsole_target *nt, char *buf)
+{
+	return sprintf(buf, "%d\n", nt->enabled);
+}
+
 static ssize_t store_local_port(struct netconsole_target *nt, const char *buf,
 				size_t count)
 {
 	spin_lock(&target_list_lock);
+	if (nt->enabled) {
+		spin_unlock(&target_list_lock);
+		return -EINVAL;
+	}
 	nt->np.local_port = simple_strtol(buf, NULL, 10);
 	spin_unlock(&target_list_lock);
 
@@ -142,6 +152,10 @@ static ssize_t store_remote_port(struct 
 				size_t count)
 {
 	spin_lock(&target_list_lock);
+	if (nt->enabled) {
+		spin_unlock(&target_list_lock);
+		return -EINVAL;
+	}
 	nt->np.remote_port = simple_strtol(buf, NULL, 10);
 	spin_unlock(&target_list_lock);
 
@@ -152,6 +166,10 @@ static ssize_t store_local_ip(struct net
 			      size_t count)
 {
 	spin_lock(&target_list_lock);
+	if (nt->enabled) {
+		spin_unlock(&target_list_lock);
+		return -EINVAL;
+	}
 	nt->np.local_ip = ntohl(in_aton(buf));
 	spin_unlock(&target_list_lock);
 
@@ -162,6 +180,10 @@ static ssize_t store_remote_ip(struct ne
 			       size_t count)
 {
 	spin_lock(&target_list_lock);
+	if (nt->enabled) {
+		spin_unlock(&target_list_lock);
+		return -EINVAL;
+	}
 	nt->np.remote_ip = ntohl(in_aton(buf));
 	spin_unlock(&target_list_lock);
 
@@ -184,12 +206,39 @@ static ssize_t store_remote_mac(struct n
 	if (i != ETH_ALEN)
 		return -EINVAL;
 	spin_lock(&target_list_lock);
+	if (nt->enabled) {
+		spin_unlock(&target_list_lock);
+		return -EINVAL;
+	}
 	memcpy(nt->np.remote_mac, input_mac, ETH_ALEN);
 	spin_unlock(&target_list_lock);
 
 	return count;
 }
 
+static ssize_t store_enabled(struct netconsole_target *nt, const char *buf,
+			    size_t count)
+{
+	int enabled = 0;
+
+	if (count >= 2 && (count != 2 || buf[count - 1] != '\n')) {
+		printk(KERN_ERR "netconsole: invalid argument: %s\n", buf);
+		return -EINVAL;
+	} else if (buf[0] == '1') {
+		enabled = 1;
+	} else if(buf[0] == '0') {
+		enabled = 0;
+	} else {
+		printk(KERN_ERR "netconsole: invalid argument: %s\n", buf);
+		return -EINVAL;
+	}
+	spin_lock(&target_list_lock);
+	nt->enabled = enabled;
+	spin_unlock(&target_list_lock);
+
+	return count;
+}
+
 struct target_attr {
 	struct attribute attr;
 	ssize_t (*show)(struct netconsole_target*, char*);
@@ -213,6 +262,8 @@ static NETCON_CLASS_ATTR(remote_ip, S_IR
 static NETCON_CLASS_ATTR(local_mac, S_IRUGO, show_local_mac, NULL);
 static NETCON_CLASS_ATTR(remote_mac, S_IRUGO | S_IWUSR,
 			 show_remote_mac, store_remote_mac);
+static NETCON_CLASS_ATTR(enabled, S_IRUGO | S_IWUSR,
+			 show_enabled, store_enabled);
 
 static struct attribute *target_attrs[] = {
 	&target_attr_id.attr,
@@ -222,6 +273,7 @@ static struct attribute *target_attrs[] 
 	&target_attr_remote_ip.attr,
 	&target_attr_local_mac.attr,
 	&target_attr_remote_mac.attr,
+	&target_attr_enabled.attr,
 	NULL
 };
 
@@ -380,6 +432,7 @@ static int add_target(char* target_confi
 	}
 
 	new_target->id = atomic_inc_return(&target_count);
+	new_target->enabled = 1;
 
 	printk(KERN_INFO "netconsole: add target: "
 	       "remote ip_addr=%d.%d.%d.%d remote port=%d\n",
@@ -421,7 +474,8 @@ static void write_msg(struct console *co
 	for(left = len; left; ) {
 		frag = min(left, MAX_PRINT_CHUNK);
 		list_for_each_entry(target, &target_list, list) {
-			netpoll_send_udp(&target->np, msg, frag);
+			if (target->enabled)
+				netpoll_send_udp(&target->np, msg, frag);
 		}
 		msg += frag;
 		left -= frag;

-- 

[RFC][PATCH -mm take4 6/6] add ioctls for adding/removing target

[Keiichi KII]

From: Keiichi KII <k-keiichi@bx.jp.nec.com>

We add ioctls for adding/removing target.
If we use NETCONSOLE_ADD_TARGET ioctl, 
we can dynamically add netconsole target.
If we use NETCONSOLE_REMOVE_TARGET ioctl,
we can dynamically remoe netconsole target.

We attach a sample program for ioctl.

Signed-off-by: Keiichi KII <k-keiichi@bx.jp.nec.com>
Signed-off-by: Takayoshi Kochi <t-kochi@bq.jp.nec.com>
---
/*
 * This software is a sample program for ioctl of netconsole.
 * You can add/remove netconsole port by using this software.
 *
 * Keiichi KII <k-keiichi@bx.jp.nec.com>
 * Copyright (C) 2007 by Keiichi KII
 * This software is under GPL version 2 of the license.
 */

#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
#include <stropts.h>
#include <fcntl.h>
#include <arpa/inet.h>
#include <net/if.h>
#include <linux/if_ether.h>
#include <linux/netconsole.h>

#define NETCONSOLE_DEV_NAME "/dev/netconsole"
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

struct command {
	char *name;
	char *options;
	int (*handle_command)(struct command* command, int argc, char* argv[]);
	void (*usage)(char *msg);
};

extern char *optarg;
extern int opterr, optind, errno;

static void generic_usage(char *msg) {
	fprintf(stderr, "Usage  : netconfig command [option] [args]\n");
	fprintf(stderr, "command: add remove help\n");
	exit(-1);
}

static int handle_command_add(struct command* command, int argc, char** argv)
{
	int i, fd, ch;
	unsigned int address;
	unsigned char mac[ETH_ALEN];
	struct netconsole_request req = {
		.netdev_name = "eth0",
		.local_port = 6665,
		.remote_port = 6666,
		.remote_mac = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF},
	};

	while ((ch = getopt(argc, argv, command->options)) != -1) {
		switch (ch) {
		case 'p':
			req.local_port = atoi(optarg);
			break;
		case 's':
			address = inet_addr(optarg);
			if (address == -1)
				(*command->usage)("invlid IP address!\n");
			req.local_ip = address;
			break;
		case 'h':
		default:
			(*command->usage)(NULL);
		}
	}
	argc -= optind;
	argv += optind;

	if (argc < 3 || argc > 4)
		(*command->usage)(NULL);

	memcpy(req.netdev_name, argv[0], IFNAMSIZ);
	address = inet_addr(argv[1]);
	if (address == -1)
		(*command->usage)("invlid IP address!\n");
	req.remote_ip = address;
	req.remote_port = atoi(argv[2]);
	if (argc == 4) {
		i = 0;
		mac[i++] = strtol(argv[3], NULL, 16);
		while ((argv[3] = strchr(argv[3], ':')) != NULL) {
			argv[3]++;
			mac[i++] = strtol(argv[3], NULL, 16);
		}
		if (i != ETH_ALEN)
			(*command->usage)("Invalid MAC address!\n");
		memcpy(req.remote_mac, mac, ETH_ALEN);
	}

	fd = open(NETCONSOLE_DEV_NAME, O_RDWR);
	if (fd < 0) {
		fprintf(stderr, "cannot open device" NETCONSOLE_DEV_NAME "\n");
		return -1;
	}

	if(ioctl(fd, NETCON_ADD_TARGET, &req) != 0)
		perror("add");
	close(fd);

	return 0;
}

static void usage_add(char *msg)
{
	if (msg != NULL)
		fprintf(stderr, "%s", msg);
	fprintf(stderr, "Usage  : netconfig add [-options] dev_name remote_ip "
		"remote_port [remote_mac]\n");
	fprintf(stderr, "options:\n");
	fprintf(stderr, "        -p local_port     :local port number\n");
	fprintf(stderr, "        -s local_up       :local IP address\n");
	exit(-1);
}

static int handle_command_remove(struct command *command,
				 int argc, char** argv)
{
	int fd, id, ch;

	while ((ch = getopt(argc, argv, command->options)) != -1) {
		switch (ch) {
		case 'h':
		default:
			(*command->usage)(NULL);
		}
	}
	argc -= optind;
	argv += optind;

	if (argc != 1)
		(*command->usage)(NULL);

	id = atoi(argv[0]);
	fd = open(NETCONSOLE_DEV_NAME, O_RDWR);
	if (fd < 0) {
		fprintf(stderr, "can't open device " NETCONSOLE_DEV_NAME "\n");
		return -1;
	}
	if(ioctl(fd, NETCON_REMOVE_TARGET, &id) != 0)
		perror("remove");
	close(fd);

	return 0;
}

static void usage_remove(char *msg)
{
	fprintf(stderr, "Usage  : netconfig remove id\n");
	exit(-1);
}

static int handle_command_help(struct command *command, int argc, char** argv)
{
	(*command->usage)(NULL);

	return 0;
}

static void usage_help(char *msg)
{
	generic_usage(NULL);
}

static struct command s_commands[] = {
	{"add", "hp:s:", handle_command_add, usage_add},
	{"remove", "hp:", handle_command_remove, usage_remove},
	{"help", NULL, handle_command_help, usage_help},
};

int main(int argc, char* argv[])
{
	int i;
	struct command *command = NULL;

	for(i = 0; i < ARRAY_SIZE(s_commands); i++) {
		if (argc <= 1)
			break;
		if (strcmp(s_commands[i].name, argv[1]) == 0) {
			command = &s_commands[i];
			break;
		}
	}

	if (command == NULL)
		generic_usage(NULL);
	argc--;
	argv++;

	if (command->handle_command)
		(*command->handle_command)(command, argc, argv);
	else {
		fprintf(stderr, "function handle_command() isn't set.");
		exit(-1);
	}
	return 0;
}
Index: mm/fs/compat_ioctl.c
===================================================================
--- mm.orig/fs/compat_ioctl.c
+++ mm/fs/compat_ioctl.c
@@ -116,6 +116,8 @@
 #include <linux/dvb/video.h>
 #include <linux/lp.h>
 
+#include <linux/netconsole.h>
+
 static int do_ioctl32_pointer(unsigned int fd, unsigned int cmd,
 			      unsigned long arg, struct file *f)
 {
Index: mm/include/linux/compat_ioctl.h
===================================================================
--- mm.orig/include/linux/compat_ioctl.h
+++ mm/include/linux/compat_ioctl.h
@@ -828,3 +828,6 @@ COMPATIBLE_IOCTL(VIDEO_GET_NAVI)
 COMPATIBLE_IOCTL(VIDEO_SET_ATTRIBUTES)
 COMPATIBLE_IOCTL(VIDEO_GET_SIZE)
 COMPATIBLE_IOCTL(VIDEO_GET_FRAME_RATE)
+/* netconsole */
+COMPATIBLE_IOCTL(NETCON_ADD_TARGET)
+ULONG_IOCTL(NETCON_REMOVE_TARGET)
Index: mm/include/linux/netconsole.h
===================================================================
--- /dev/null
+++ mm/include/linux/netconsole.h
@@ -0,0 +1,18 @@
+#ifndef __NETCONSOLE_H
+#define __NETCONSOLE_H
+
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+struct netconsole_request {
+	__u32 id;
+	__u8 netdev_name[IFNAMSIZ];
+	__u8 remote_mac[ETH_ALEN];
+	__u32 local_ip, remote_ip;
+	__u16 local_port, remote_port;
+};
+
+#define NETCON_ADD_TARGET _IOW(0xAE, 4, struct netconsole_request)
+#define NETCON_REMOVE_TARGET _IOW(0xAE, 5, int)
+
+#endif /* __NETCONSOLE_H */
Index: mm/drivers/net/netconsole.c
===================================================================
--- mm.orig/drivers/net/netconsole.c
+++ mm/drivers/net/netconsole.c
@@ -47,6 +47,7 @@
 #include <linux/netpoll.h>
 #include <linux/miscdevice.h>
 #include <linux/inet.h>
+#include <linux/netconsole.h>
 
 MODULE_AUTHOR("Maintainer: Matt Mackall <mpm@selenic.com>");
 MODULE_DESCRIPTION("Console driver for network interfaces");
@@ -313,6 +314,64 @@ static void release_target(struct kobjec
 	remove_target(nt);
 }
 
+static int netconsole_ioctl(struct inode *inode, struct file *file,
+			    unsigned int cmd, unsigned long arg)
+{
+	int id, count;
+	char config[256];
+	char *cur;
+	struct netconsole_request req;
+	struct netconsole_target *nt, *tmp;
+	void __user *argp = (void __user *)arg;
+
+	switch (cmd) {
+	case NETCON_ADD_TARGET:
+		printk(KERN_INFO "netconsole: cmd=NETCON_ADD_TARGET\n");
+		if (copy_from_user(&req, argp, sizeof(req)))
+			return -EFAULT;
+		cur = config;
+		count = sprintf(cur, "%d@", req.local_port);
+		cur += count;
+		if (req.local_ip)
+			count = sprintf(cur, "%d.%d.%d.%d/",
+					NIPQUAD(req.local_ip));
+		else
+			count = sprintf(cur, "/");
+		cur += count;
+		count = sprintf(cur, "%s,", req.netdev_name);
+		cur += count;
+		count = sprintf(cur, "%d@", req.remote_port);
+		cur += count;
+		count = sprintf(cur, "%d.%d.%d.%d/",
+				NIPQUAD(req.remote_ip));
+		cur += count;
+		count = sprintf(cur, "%02x:%02x:%02x:%02x:%02x:%02x",
+				req.remote_mac[0], req.remote_mac[1],
+				req.remote_mac[2], req.remote_mac[3],
+				req.remote_mac[4], req.remote_mac[5]);
+		printk(KERN_INFO "count = %d config=[%s]\n", count, config);
+		if (add_target(config))
+			return -EINVAL;
+		break;
+	case NETCON_REMOVE_TARGET:
+		printk(KERN_INFO "netconsole: cmd=NETCON_REMOVE_TARGET\n");
+		if (copy_from_user(&id, argp, sizeof(int)))
+			return -EFAULT;
+		printk(KERN_INFO "netconsole: id=%d\n", id);
+		list_for_each_entry_safe(nt, tmp, &target_list, list) {
+			if (nt->id == id) {
+				kobject_unregister(&nt->obj);
+				break;
+			}
+		}
+		break;
+	default:
+		return -ENOTTY;
+	}
+
+	return 0;
+}
+
 static struct sysfs_ops target_sysfs_ops = {
 	.show = show_target_attr,
 	.store = store_target_attr
@@ -324,9 +383,14 @@ static struct kobj_type target_ktype = {
 	.default_attrs = target_attrs,
 };
 
+static struct file_operations miscdev_fops = {
+	.ioctl = netconsole_ioctl,
+};
+
 static struct miscdevice netconsole_miscdev = {
 	.minor = MISC_DYNAMIC_MINOR,
 	.name = "netconsole",
+	.fops = &miscdev_fops,
 };
 
 static struct notifier_block netconsole_notifier = {

-- 

Re: [RFC][PATCH -mm take4 6/6] add ioctls for adding/removing target

[Andrew Morton]

On Wed, 18 Apr 2007 21:14:55 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:

> From: Keiichi KII <k-keiichi@bx.jp.nec.com>
> 
> We add ioctls for adding/removing target.
> If we use NETCONSOLE_ADD_TARGET ioctl, 
> we can dynamically add netconsole target.
> If we use NETCONSOLE_REMOVE_TARGET ioctl,
> we can dynamically remoe netconsole target.
> 
> ...
>
> --- mm.orig/drivers/net/netconsole.c
> +++ mm/drivers/net/netconsole.c
> @@ -47,6 +47,7 @@
>  #include <linux/netpoll.h>
>  #include <linux/miscdevice.h>
>  #include <linux/inet.h>
> +#include <linux/netconsole.h>
>  
>  MODULE_AUTHOR("Maintainer: Matt Mackall <mpm@selenic.com>");
>  MODULE_DESCRIPTION("Console driver for network interfaces");
> @@ -313,6 +314,64 @@ static void release_target(struct kobjec
>  	remove_target(nt);
>  }
>  
> +static int netconsole_ioctl(struct inode *inode, struct file *file,
> +			    unsigned int cmd, unsigned long arg)
> +{
> +	int id, count;
> +	char config[256];
> +	char *cur;
> +	struct netconsole_request req;
> +	struct netconsole_target *nt, *tmp;
> +	void __user *argp = (void __user *)arg;
> +
> +	switch (cmd) {
> +	case NETCON_ADD_TARGET:
> +		printk(KERN_INFO "netconsole: cmd=NETCON_ADD_TARGET\n");
> +		if (copy_from_user(&req, argp, sizeof(req)))
> +			return -EFAULT;
> +		cur = config;
> +		count = sprintf(cur, "%d@", req.local_port);
> +		cur += count;
> +		if (req.local_ip)
> +			count = sprintf(cur, "%d.%d.%d.%d/",
> +					NIPQUAD(req.local_ip));
> +		else
> +			count = sprintf(cur, "/");
> +		cur += count;
> +		count = sprintf(cur, "%s,", req.netdev_name);
> +		cur += count;
> +		count = sprintf(cur, "%d@", req.remote_port);
> +		cur += count;
> +		count = sprintf(cur, "%d.%d.%d.%d/",
> +				NIPQUAD(req.remote_ip));
> +		cur += count;
> +		count = sprintf(cur, "%02x:%02x:%02x:%02x:%02x:%02x",
> +				req.remote_mac[0], req.remote_mac[1],
> +				req.remote_mac[2], req.remote_mac[3],
> +				req.remote_mac[4], req.remote_mac[5]);
> +		printk(KERN_INFO "count = %d config=[%s]\n", count, config);
> +		if (add_target(config))
> +			return -EINVAL;
> +		break;
> +	case NETCON_REMOVE_TARGET:
> +		printk(KERN_INFO "netconsole: cmd=NETCON_REMOVE_TARGET\n");
> +		if (copy_from_user(&id, argp, sizeof(int)))
> +			return -EFAULT;
> +		printk(KERN_INFO "netconsole: id=%d\n", id);
> +		list_for_each_entry_safe(nt, tmp, &target_list, list) {
> +			if (nt->id == id) {
> +				kobject_unregister(&nt->obj);
> +				break;
> +			}
> +		}
> +		break;
> +	default:
> +		return -ENOTTY;
> +	}
> +
> +	return 0;
> +}
> +
>  static struct sysfs_ops target_sysfs_ops = {
>  	.show = show_target_attr,
>  	.store = store_target_attr
> @@ -324,9 +383,14 @@ static struct kobj_type target_ktype = {
>  	.default_attrs = target_attrs,
>  };
>  
> +static struct file_operations miscdev_fops = {
> +	.ioctl = netconsole_ioctl,
> +};
> +
>  static struct miscdevice netconsole_miscdev = {
>  	.minor = MISC_DYNAMIC_MINOR,
>  	.name = "netconsole",
> +	.fops = &miscdev_fops,
>  };
>  
>  static struct notifier_block netconsole_notifier = {

We'll need to wake up the net guys to get an opinion here.  Using an
ioctl() against a miscdev is rather untypical for networking.  I'd expect
they'd prefer to see a netlink-based interface to userspace.

Re: [RFC][PATCH -mm take4 6/6] add ioctls for adding/removing target

[Stephen Hemminger]

On Thu, 19 Apr 2007 21:16:30 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 18 Apr 2007 21:14:55 +0900 Keiichi KII <k-keiichi@bx.jp.nec.com> wrote:
> 
> > From: Keiichi KII <k-keiichi@bx.jp.nec.com>
> > 
> > We add ioctls for adding/removing target.
> > If we use NETCONSOLE_ADD_TARGET ioctl, 
> > we can dynamically add netconsole target.
> > If we use NETCONSOLE_REMOVE_TARGET ioctl,
> > we can dynamically remoe netconsole target.
> > 
> > ...
> >
> > --- mm.orig/drivers/net/netconsole.c
> > +++ mm/drivers/net/netconsole.c
> > @@ -47,6 +47,7 @@
> >  #include <linux/netpoll.h>
> >  #include <linux/miscdevice.h>
> >  #include <linux/inet.h>
> > +#include <linux/netconsole.h>
> >  
> >  MODULE_AUTHOR("Maintainer: Matt Mackall <mpm@selenic.com>");
> >  MODULE_DESCRIPTION("Console driver for network interfaces");
> > @@ -313,6 +314,64 @@ static void release_target(struct kobjec
> >  	remove_target(nt);
> >  }
> >  
> > +static int netconsole_ioctl(struct inode *inode, struct file *file,
> > +			    unsigned int cmd, unsigned long arg)
> > +{
> > +	int id, count;
> > +	char config[256];
> > +	char *cur;
> > +	struct netconsole_request req;
> > +	struct netconsole_target *nt, *tmp;
> > +	void __user *argp = (void __user *)arg;
> > +
> > +	switch (cmd) {
> > +	case NETCON_ADD_TARGET:
> > +		printk(KERN_INFO "netconsole: cmd=NETCON_ADD_TARGET\n");
> > +		if (copy_from_user(&req, argp, sizeof(req)))
> > +			return -EFAULT;
> > +		cur = config;
> > +		count = sprintf(cur, "%d@", req.local_port);
> > +		cur += count;
> > +		if (req.local_ip)
> > +			count = sprintf(cur, "%d.%d.%d.%d/",
> > +					NIPQUAD(req.local_ip));
> > +		else
> > +			count = sprintf(cur, "/");
> > +		cur += count;
> > +		count = sprintf(cur, "%s,", req.netdev_name);
> > +		cur += count;
> > +		count = sprintf(cur, "%d@", req.remote_port);
> > +		cur += count;
> > +		count = sprintf(cur, "%d.%d.%d.%d/",
> > +				NIPQUAD(req.remote_ip));
> > +		cur += count;
> > +		count = sprintf(cur, "%02x:%02x:%02x:%02x:%02x:%02x",
> > +				req.remote_mac[0], req.remote_mac[1],
> > +				req.remote_mac[2], req.remote_mac[3],
> > +				req.remote_mac[4], req.remote_mac[5]);
> > +		printk(KERN_INFO "count = %d config=[%s]\n", count, config);
> > +		if (add_target(config))
> > +			return -EINVAL;
> > +		break;
> > +	case NETCON_REMOVE_TARGET:
> > +		printk(KERN_INFO "netconsole: cmd=NETCON_REMOVE_TARGET\n");
> > +		if (copy_from_user(&id, argp, sizeof(int)))
> > +			return -EFAULT;
> > +		printk(KERN_INFO "netconsole: id=%d\n", id);
> > +		list_for_each_entry_safe(nt, tmp, &target_list, list) {
> > +			if (nt->id == id) {
> > +				kobject_unregister(&nt->obj);
> > +				break;
> > +			}
> > +		}
> > +		break;
> > +	default:
> > +		return -ENOTTY;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static struct sysfs_ops target_sysfs_ops = {
> >  	.show = show_target_attr,
> >  	.store = store_target_attr
> > @@ -324,9 +383,14 @@ static struct kobj_type target_ktype = {
> >  	.default_attrs = target_attrs,
> >  };
> >  
> > +static struct file_operations miscdev_fops = {
> > +	.ioctl = netconsole_ioctl,
> > +};
> > +
> >  static struct miscdevice netconsole_miscdev = {
> >  	.minor = MISC_DYNAMIC_MINOR,
> >  	.name = "netconsole",
> > +	.fops = &miscdev_fops,
> >  };
> >  
> >  static struct notifier_block netconsole_notifier = {
> 
> We'll need to wake up the net guys to get an opinion here.  Using an
> ioctl() against a miscdev is rather untypical for networking.  I'd expect
> they'd prefer to see a netlink-based interface to userspace.

Should't this just be a network ioctl against an UDP (AF_INET, SOCK_DGRAM) socket?
Also consider netconsole over IPV6 for future enhancement.

Re: [RFC][PATCH -mm take4 6/6] add ioctls for adding/removing target

[David Miller]

From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Thu, 19 Apr 2007 22:39:09 -0700

> Should't this just be a network ioctl against an UDP (AF_INET,
> SOCK_DGRAM) socket?  Also consider netconsole over IPV6 for future
> enhancement.

If it's on a socket use a socket option :-)