Skip to contentSkip to conversation

proposal for dynamic configurable netconsole

RFC PATCH In review Keiichi KII 5 participants 25 messages Apr 18, 2007 – Apr 26, 2007

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover proposal for dynamic configurable netconsole

From: Keiichi KII <[email protected]>

The netconsole is a very useful module for collecting kernel message under certain circumstances(e.g. disk logging fails, serial port is unavailable).

But current netconsole is not flexible. For example, if you want to change ip address for logging agent, in the case of built-in netconsole, you can't change config except for changing boot parameter and rebooting your system, or in the case of module netconsole, you need to remove it and reload with different parameters.

By adopting my patches, the current netconsole becomes a little complex. But the kernel messages(especially panic messages) is significant information
to solve bugs and troubles promptly and we have been losing serial console
port with PCs and Servers.

I think that we need the environment in which we can collect kernel messages flexibly.

So, I propose the following extended features for netconsole.

1) support for multiple logging agents.
2) add interface to access each parameter of netconsole using sysfs.

[changes since take3]
-changing kernel base from 2.6.21-rc3-mm2 to 2.6.21-rc6-mm1. -introducing CONFIG_NETCONSOLE_DYNCON.
-cleanup

Your comments are very welcome.

---

#1 lore raw

RFC PATCH 1/6 marking __init

From: Keiichi KII <[email protected]>

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

---

drivers/net/netconsole.c +2 -2
Index: 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;
 
#2 lore raw

RFC PATCH 2/6 support multiple logging

In reply to Keiichi KII

From: Keiichi KII <[email protected]>

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.

---

drivers/net/netconsole.c +122 -13
Index: 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);
drivers/net/Kconfig +8 -0
Index: 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
#3 lore raw

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

Quoted 10 lines · On Wed, 18 Apr 2007 21:06:41 +0900 Keiichi KII <[email protected]> wrote:

From: Keiichi KII <[email protected]>

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.

Quoted 7 lines

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

#4 lore raw

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

Quoted 1 line

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.

Signature · 3 lines
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
#5 lore raw

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

In reply to Keiichi KII
Quoted 39 lines · On Wed, 18 Apr 2007 21:06:41 +0900 Keiichi KII <[email protected]> wrote:

From: Keiichi KII <[email protected]>

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.

---
Index: mm/drivers/net/netconsole.c
===================================================================

drivers/net/netconsole.c +3 -1
@@ -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;

drivers/net/netconsole.c static struct netpoll np = { · line 79

Quoted 10 lines
drivers/net/netconsole.c +10 -0
+	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()

drivers/net/Kconfig renamed from drivers/net/Kconfig~netconsole-support-multiple-logging-fix +5 -3
diff -puN drivers/net/Kconfig~netconsole-support-multiple-logging-fix 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
drivers/net/netconsole.c renamed from drivers/net/netconsole.c~netconsole-support-multiple-logging-fix +20 -28

90 lines not on this page · raw message

_

#6 lore raw

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

Quoted 4 lines

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.

Signature · 3 lines
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
#7 lore raw

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

Quoted 8 lines · On Fri, 20 Apr 2007 18:51:13 +0900 Keiichi KII <[email protected]> wrote:
Quoted 4 lines

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??

#8 lore raw

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

Quoted 15 lines · Andrew Morton
Quoted 8 lines · On Fri, 20 Apr 2007 18:51:13 +0900 Keiichi KII <[email protected]> wrote:
Quoted 4 lines

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.

Quoted 7 lines

- 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.

Quoted 7 lines

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...

Quoted 8 lines

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]

Signature · 1 line
Mathematics is the supreme nostalgia of our time.
#9 lore raw

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

Quoted 42 lines · On Fri, 20 Apr 2007 13:21:10 -0500 Matt Mackall <[email protected]> wrote:
Quoted 15 lines · Andrew Morton
Quoted 8 lines · On Fri, 20 Apr 2007 18:51:13 +0900 Keiichi KII <[email protected]> wrote:
Quoted 4 lines

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.

Quoted 7 lines

- 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.

Quoted 7 lines

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...

Quoted 8 lines

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??

Quoted 8 lines

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.

#10 lore raw

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

Quoted 6 lines
Quoted 6 lines

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:

Quoted 8 lines
Quoted 2 lines

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?

Signature · 3 lines
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
#11 lore raw

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

In reply to Andrew Morton
Quoted 6 lines
Quoted 3 lines · On Fri, 20 Apr 2007 18:51:13 +0900 Keiichi KII <[email protected]> wrote:
Quoted 3 lines

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

Quoted 9 lines
Quoted 3 lines
Quoted 1 line

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?

Signature · 3 lines
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
#12 lore raw

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

Quoted 11 lines · On Tue, 24 Apr 2007 17:14:28 +0900 Keiichi KII <[email protected]> wrote:
Quoted 11 lines
Quoted 7 lines · On Fri, 20 Apr 2007 18:51:13 +0900 Keiichi KII <[email protected]> wrote:
Quoted 4 lines

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.

Quoted 9 lines
Quoted 5 lines

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?

#13 lore raw

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

Quoted 19 lines · On Tue, 24 Apr 2007 01:29:02 -0700 Andrew Morton <[email protected]>…
Quoted 17 lines · On Tue, 24 Apr 2007 17:14:28 +0900 Keiichi KII <[email protected]> wrote:
Quoted 16 lines
Quoted 7 lines · On Fri, 20 Apr 2007 18:51:13 +0900 Keiichi KII <[email protected]> wrote:
Quoted 4 lines

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

Quoted 7 lines
Quoted 3 lines

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.

#14 lore raw

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

Quoted 10 lines
Quoted 10 lines
Quoted 7 lines
Quoted 5 lines

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.

Quoted 11 lines

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.

Signature · 3 lines
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
maintainer
#15 lore raw

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

From: Keiichi KII <[email protected]>
Date: Thu, 26 Apr 2007 13:02:04 +0900

Quoted 5 lines

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.

#16 lore raw

RFC PATCH 3/6 add interface for netconsole using sysfs

In reply to Keiichi KII

From: Keiichi KII <[email protected]>

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/

...

---

drivers/net/netconsole.c +217 -1
Index: 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 <[email protected]>");
 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);
#17 lore raw

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

Quoted 9 lines · On Wed, 18 Apr 2007 21:08:45 +0900 Keiichi KII <[email protected]> wrote:
drivers/net/netconsole.c +9 -0
+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;
+}

drivers/net/netconsole.c static DEFINE_SPINLOCK(target_list_lock) · line 146

Quoted 10 lines
drivers/net/netconsole.c +10 -0
+
+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.

#18 lore raw

RFC PATCH 4/6 using symlink for the net_device

In reply to Keiichi KII

From: Keiichi KII <[email protected]>

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/

...

---

drivers/net/netconsole.c +0 -0
Index: 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

#19 lore raw

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

Quoted 3 lines · On Wed, 18 Apr 2007 21:11:14 +0900 Keiichi KII <[email protected]> wrote:

From: Keiichi KII <[email protected]>

We use symbolic link for net_device.

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

Please also cc [email protected] on all networking-related patches.

Review

Quoted 3 lines
(quoted diff) +3 -0
+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).

Quoted 36 lines

static int miscdev_configured;

(quoted diff) +24 -1
@@ -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;

Review static struct miscdevice netconsole_misc · line 318

Quoted 10 lines
(quoted diff) +10 -0
+	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()

Quoted 6 lines
(quoted diff) +6 -0
+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;

Review static struct miscdevice netconsole_misc · line 334

Quoted 10 lines
(quoted diff) +10 -0
+	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.

Quoted 16 lines
(quoted diff) +15 -0
+			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;
 }
#20 lore raw

RFC PATCH 5/6 switch function of netpoll

In reply to Keiichi KII

From: Keiichi KII <[email protected]>

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/

...

---

drivers/net/netconsole.c +55 -1
Index: 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;
#21 lore raw

RFC PATCH 6/6 add ioctls for adding/removing target

In reply to Keiichi KII

From: Keiichi KII <[email protected]>

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.

---
/*
* This software is a sample program for ioctl of netconsole.
* You can add/remove netconsole port by using this software. *
* Keiichi KII <[email protected]>
* 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;
}
fs/compat_ioctl.c +2 -0
Index: 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)
 {
include/linux/compat_ioctl.h +3 -0
Index: 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)
include/linux/netconsole.h new file +18 -0
Index: 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 */
drivers/net/netconsole.c +64 -0

85 lines not on this page · raw message

#22 lore raw

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

Quoted 90 lines · On Wed, 18 Apr 2007 21:14:55 +0900 Keiichi KII <[email protected]> wrote:

From: Keiichi KII <[email protected]>

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.

...

drivers/net/netconsole.c +60 -0
@@ -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 <[email protected]>");
 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 = {

drivers/net/netconsole.c static struct kobj_type target_ktype = { · line 396

Quoted 10 lines
drivers/net/netconsole.c +4 -0
+	.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.

#23 lore raw

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

Quoted 101 lines · On Thu, 19 Apr 2007 21:16:30 -0700 Andrew Morton <[email protected]>…
Quoted 99 lines · On Wed, 18 Apr 2007 21:14:55 +0900 Keiichi KII <[email protected]> wrote:

From: Keiichi KII <[email protected]>

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.

...

drivers/net/netconsole.c +64 -0
@@ -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 <[email protected]>");
 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,
 };
 
Quoted 5 lines

drivers/net/netconsole.c static struct kobj_type target_ktype = { · line 396

Quoted 1 line
drivers/net/netconsole.c +0 -0
 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.

maintainer
#24 lore raw

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

From: Stephen Hemminger <[email protected]>
Date: Thu, 19 Apr 2007 22:39:09 -0700

Quoted 3 lines

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 :-)