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 reload netconsole module.
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.
This patch is for linux-2.6.19 and is divided to each function.
Your comments are very welcome.
Signed-off-by: Keiichi KII <[email protected]>
---
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
From: Keiichi KII <[email protected]>
This patch contains the following cleanups.
- add __init for initialization functions(option_setup() and init_netconsole()).
- define name of magic number.
Signed-off-by: Keiichi KII <[email protected]>
---
--- linux-2.6.19/drivers/net/netconsole.c 2006-12-06 14:37:06.985584500 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.cleanup 2006-12-06
14:34:52.561183500 +0900
@@ -50,8 +50,14 @@ MODULE_AUTHOR("Maintainer: Matt Mackall
MODULE_DESCRIPTION("Console driver for network interfaces");
MODULE_LICENSE("GPL");
-static char config[256];
-module_param_string(netconsole, config, 256, 0);
+enum {
+ MAX_PRINT_CHUNK = 1000,
+ MAX_CONFIG_LENGTH = 256,
+};
+
+static char config[MAX_CONFIG_LENGTH];
+
+module_param_string(netconsole, config, MAX_CONFIG_LENGTH, 0);
MODULE_PARM_DESC(netconsole, "
netconsole=[src-port]@[src-ip]/[dev],[tgt-port]@<tgt-ip>/[tgt-macaddr]\n");
static struct netpoll np = {
@@ -62,9 +68,8 @@ static struct netpoll np = {
.remote_mac = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
.drop = netpoll_queue,
};
-static int configured = 0;
-#define MAX_PRINT_CHUNK 1000
+static int configured = 0;
static void write_msg(struct console *con, const char *msg, unsigned int len)
{
@@ -75,14 +80,12 @@ static void write_msg(struct console *co
return;
local_irq_save(flags);
-
for(left = len; left; ) {
frag = min(left, MAX_PRINT_CHUNK);
netpoll_send_udp(&np, msg, frag);
msg += frag;
left -= frag;
}
-
local_irq_restore(flags);
}
@@ -92,7 +95,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;
@@ -100,7 +103,7 @@ static int option_setup(char *opt)
__setup("netconsole=", option_setup);
-static int init_netconsole(void)
+static int __init init_netconsole(void)
{
if(strlen(config))
option_setup(config);
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
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
| |--- remove [-w-------] if you write something to "remove",
| | this port is removed.
| |--- dev_name [r--r--r--] network interface name
| |--- local_ip [rw-r--r--] source IP to use, writable
| |--- local_port [rw-r--r--] source port number for UDP packets, writable
| |--- local_mac [r--r--r--] source MAC address
| |--- remote_ip [rw-r--r--] port number for logging agent, writable
| |--- remote_port [rw-r--r--] IP address for logging agent, writable
| ---- remote_mac [rw-r--r--] MAC address for logging agent, writable
|--- port2/
|--- port3/
...
Signed-off-by: Keiichi KII <[email protected]>
---
--- linux-2.6.19/drivers/net/netconsole.c 2006-12-06 14:37:26.842825500 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.sysfs 2006-12-06
13:32:47.488381000 +0900
@@ -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");
@@ -53,6 +55,7 @@ MODULE_LICENSE("GPL");
enum {
MAX_PRINT_CHUNK = 1000,
MAX_CONFIG_LENGTH = 256,
+ MAC_ADDR_DIGIT = 6,
};
static char config[MAX_CONFIG_LENGTH];
@@ -62,19 +65,214 @@ MODULE_PARM_DESC(netconsole, " netconsol
struct netconsole_device {
struct list_head list;
+ struct kobject obj;
spinlock_t netpoll_lock;
int id;
struct netpoll np;
};
+struct netcon_dev_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct netconsole_device*, char*);
+ ssize_t (*store)(struct netconsole_device*, const char*,
+ size_t count);
+};
+
static int add_netcon_dev(const char*);
+static void setup_netcon_dev_sysfs(struct netconsole_device*);
static void cleanup_netconsole(void);
static void netcon_dev_cleanup(struct netconsole_device *nd);
+static int netcon_miscdev_configured = 0;
+
static LIST_HEAD(active_netconsole_dev);
static DEFINE_SPINLOCK(netconsole_dev_list_lock);
+#define SHOW_CLASS_ATTR(field, type, format, ...) \
+static ssize_t show_##field(type, char *buf) \
+{ \
+ return sprintf(buf, format, __VA_ARGS__); \
+} \
+
+SHOW_CLASS_ATTR(id, struct netconsole_device *nd, "%d\n", nd->id);
+SHOW_CLASS_ATTR(dev_name, struct netconsole_device *nd,
+ "%s\n", nd->np.dev_name);
+SHOW_CLASS_ATTR(local_port, struct netconsole_device *nd,
+ "%d\n", nd->np.local_port);
+SHOW_CLASS_ATTR(remote_port, struct netconsole_device *nd,
+ "%d\n", nd->np.remote_port);
+SHOW_CLASS_ATTR(local_ip, struct netconsole_device *nd,
+ "%d.%d.%d.%d\n", HIPQUAD(nd->np.local_ip));
+SHOW_CLASS_ATTR(remote_ip, struct netconsole_device *nd,
+ "%d.%d.%d.%d\n", HIPQUAD(nd->np.remote_ip));
+SHOW_CLASS_ATTR(local_mac, struct netconsole_device *nd,
+ "%02x:%02x:%02x:%02x:%02x:%02x\n",
+ nd->np.local_mac[0], nd->np.local_mac[1], nd->np.local_mac[2],
+ nd->np.local_mac[3], nd->np.local_mac[4], nd->np.local_mac[5]);
+SHOW_CLASS_ATTR(remote_mac, struct netconsole_device *nd,
+ "%02x:%02x:%02x:%02x:%02x:%02x\n",
+ nd->np.remote_mac[0], nd->np.remote_mac[1],
+ nd->np.remote_mac[2], nd->np.remote_mac[3],
+ nd->np.remote_mac[4], nd->np.remote_mac[5]);
+
+static ssize_t store_local_port(struct netconsole_device *nd, const char *buf,
+ size_t count)
+{
+ spin_lock(&nd->netpoll_lock);
+ nd->np.local_port = simple_strtol(buf, NULL, 10);
+ spin_unlock(&nd->netpoll_lock);
+
+ return count;
+}
+
+static ssize_t store_remote_port(struct netconsole_device *nd, const char *buf,
+ size_t count)
+{
+ spin_lock(&nd->netpoll_lock);
+ nd->np.remote_port = simple_strtol(buf, NULL, 10);
+ spin_unlock(&nd->netpoll_lock);
+
+ return count;
+}
+
+static ssize_t store_local_ip(struct netconsole_device *nd, const char *buf,
+ size_t count)
+{
+ spin_lock(&nd->netpoll_lock);
+ nd->np.local_ip = ntohl(in_aton(buf));
+ spin_unlock(&nd->netpoll_lock);
+
+ return count;
+}
+
+static ssize_t store_remote_ip(struct netconsole_device *nd, const char *buf,
+ size_t count)
+{
+ spin_lock(&nd->netpoll_lock);
+ nd->np.remote_ip = ntohl(in_aton(buf));
+ spin_unlock(&nd->netpoll_lock);
+
+ return count;
+}
+
+static ssize_t store_remote_mac(struct netconsole_device *nd, const char *buf,
+ size_t count)
+{
+ unsigned char input_mac[MAC_ADDR_DIGIT] =
+ {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+ const char *cur=buf;
+ char *delim;
+ int i = 0;
+
+ for(i=0; i < MAC_ADDR_DIGIT; i++) {
+ input_mac[i] = simple_strtol(cur, NULL, 16);
+ if (i != MAC_ADDR_DIGIT - 1 &&
+ (delim = strchr(cur, ':')) == NULL) {
+ return -EINVAL;
+ }
+ cur=delim+1;
+ }
+ spin_lock(&nd->netpoll_lock);
+ memcpy(nd->np.remote_mac, input_mac, MAC_ADDR_DIGIT);
+ spin_unlock(&nd->netpoll_lock);
+
+ return count;
+}
+
+static ssize_t store_remove(struct netconsole_device *nd, const char *buf,
+ size_t count)
+{
+ kobject_unregister(&nd->obj);
+ return count;
+}
+
+#define NETCON_CLASS_ATTR(_name, _mode, _show, _store) \
+struct netcon_dev_attr netcon_dev_attr_##_name = \
+ __ATTR(_name, _mode, _show, _store)
+
+static NETCON_CLASS_ATTR(id, S_IRUGO, show_id, NULL);
+static NETCON_CLASS_ATTR(dev_name, S_IRUGO, show_dev_name, 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 NETCON_CLASS_ATTR(remove, S_IWUSR, NULL, store_remove);
+
+static struct attribute *netcon_dev_attrs[] = {
+ &netcon_dev_attr_id.attr,
+ &netcon_dev_attr_dev_name.attr,
+ &netcon_dev_attr_local_port.attr,
+ &netcon_dev_attr_remote_port.attr,
+ &netcon_dev_attr_local_ip.attr,
+ &netcon_dev_attr_remote_ip.attr,
+ &netcon_dev_attr_local_mac.attr,
+ &netcon_dev_attr_remote_mac.attr,
+ &netcon_dev_attr_remove.attr,
+ NULL
+};
+
+static ssize_t show_netcon_dev_attr(struct kobject *kobj,
+ struct attribute *attr,
+ char *buffer)
+{
+ struct netcon_dev_attr *na = container_of(attr, struct netcon_dev_attr,
+ attr);
+ struct netconsole_device * nd =
+ container_of(kobj, struct netconsole_device, obj);
+ if (na->show) {
+ return na->show(nd, buffer);
+ } else {
+ return -EACCES;
+ }
+}
+
+static ssize_t store_netcon_dev_attr(struct kobject *kobj,
+ struct attribute *attr,
+ const char *buffer, size_t count)
+{
+ struct netcon_dev_attr *na = container_of(attr, struct netcon_dev_attr,
+ attr);
+ struct netconsole_device *nd =
+ container_of(kobj, struct netconsole_device, obj);
+ if (na->store) {
+ return na->store(nd, buffer, count);
+ } else {
+ return -EACCES;
+ }
+}
+
+static void netcon_dev_release(struct kobject *kobj)
+{
+ struct netconsole_device *nd =
+ container_of(kobj, struct netconsole_device, obj);
+
+ netcon_dev_cleanup(nd);
+}
+
+static struct sysfs_ops netcon_dev_sysfs_ops = {
+ .show = show_netcon_dev_attr,
+ .store = store_netcon_dev_attr
+};
+
+static struct kobj_type netcon_dev_ktype = {
+ .release = netcon_dev_release,
+ .sysfs_ops = &netcon_dev_sysfs_ops,
+ .default_attrs = netcon_dev_attrs,
+};
+
+static struct miscdevice netcon_miscdev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "netconsole",
+};
+
static struct netpoll np = {
.name = "netconsole",
.dev_name = "eth0",
@@ -129,6 +327,8 @@ static int add_netcon_dev(const char* ta
list_add(&new_dev->list, &active_netconsole_dev);
spin_unlock(&netconsole_dev_list_lock);
+ setup_netcon_dev_sysfs(new_dev);
+
kfree(netcon_dev_config);
return 0;
@@ -138,6 +338,14 @@ static int add_netcon_dev(const char* ta
return -EINVAL;
}
+static void setup_netcon_dev_sysfs(struct netconsole_device *target)
+{
+ kobject_set_name(&target->obj, "port%d", target->id);
+ target->obj.parent = &netcon_miscdev.class->kobj;
+ target->obj.ktype = &netcon_dev_ktype;
+ kobject_register(&target->obj);
+}
+
static void netcon_dev_cleanup(struct netconsole_device *nd)
{
spin_lock(&netconsole_dev_list_lock);
@@ -191,6 +399,13 @@ static int __init init_netconsole(void)
char *p;
char *tmp = config;
+ if (misc_register(&netcon_miscdev)) {
+ printk(KERN_INFO
+ "netconsole: unable netconsole misc device\n");
+ return -EIO;
+ } else {
+ netcon_miscdev_configured = 1;
+ }
register_console(&netconsole);
if(!strlen(config)) {
@@ -215,8 +430,13 @@ static void cleanup_netconsole(void)
struct netconsole_device *dev, *tmp;
unregister_console(&netconsole);
+
list_for_each_entry_safe(dev, tmp, &active_netconsole_dev, list) {
- netcon_dev_cleanup(dev);
+ kobject_unregister(&dev->obj);
+ }
+
+ if (netcon_miscdev_configured) {
+ misc_deregister(&netcon_miscdev);
}
}
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
From: Keiichi KII <[email protected]>
This patch contains switch function of netpoll.
if "enable" attribute of certain port is '1', this port is used.
if "enable" attribute of certain port is '0', this port isn't used.
active_netconsole_dev list manages a list of active ports.
inactive_netconsole_dev list manages a list of inactive ports.
If you write '0' to "enable" attribute of a port included in
active_netconsole_dev_list, This port moves from active_netconsole_dev to
inactive_netconsole_dev and won't used to send kernel message.
-+- /sys/class/misc/
|-+- netconsole/
|-+- port1/
| |--- id [r--r--r--] id
| |--- enable [rw-r--r--] 0: disable, 1: enable, writable
| ...
|--- port2/
...
Signed-off-by: Keiichi KII <[email protected]>
---
--- linux-2.6.19/drivers/net/netconsole.c 2006-12-06 14:37:26.858826500 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.switch 2006-12-06
13:32:56.744959500 +0900
@@ -86,9 +86,25 @@ static void netcon_dev_cleanup(struct ne
static int netcon_miscdev_configured = 0;
static LIST_HEAD(active_netconsole_dev);
+static LIST_HEAD(inactive_netconsole_dev);
static DEFINE_SPINLOCK(netconsole_dev_list_lock);
+static ssize_t show_enable(struct netconsole_device *nd, char *buf) {
+ struct netconsole_device *dev;
+
+ spin_lock(&netconsole_dev_list_lock);
+ list_for_each_entry(dev, &active_netconsole_dev, list) {
+ if (dev->id == nd->id) {
+ spin_unlock(&netconsole_dev_list_lock);
+ return sprintf(buf, "1\n");
+ }
+ }
+ spin_unlock(&netconsole_dev_list_lock);
+
+ return sprintf(buf, "0\n");
+}
+
#define SHOW_CLASS_ATTR(field, type, format, ...) \
static ssize_t show_##field(type, char *buf) \
{ \
@@ -180,6 +196,36 @@ static ssize_t store_remote_mac(struct n
return count;
}
+static ssize_t store_enable(struct netconsole_device *nd, const char *buf,
+ size_t count)
+{
+ struct netconsole_device *dev, *tmp;
+ struct list_head *src, *dst;
+
+ if (strncmp(buf, "1", 1) == 0) {
+ src = &inactive_netconsole_dev;
+ dst = &active_netconsole_dev;
+ } else if(strncmp(buf, "0", 1) == 0) {
+ src = &active_netconsole_dev;
+ dst = &inactive_netconsole_dev;
+ } else {
+ printk(KERN_INFO "netconsole: invalid argument: %s\n", buf);
+ return -EINVAL;
+ }
+
+ spin_lock(&netconsole_dev_list_lock);
+ list_for_each_entry_safe(dev, tmp, src, list) {
+ if (dev->id == nd->id) {
+ list_del(&nd->list);
+ list_add(&nd->list, dst);
+ break;
+ }
+ }
+ spin_unlock(&netconsole_dev_list_lock);
+
+ return count;
+}
+
static ssize_t store_remove(struct netconsole_device *nd, const char *buf,
size_t count)
{
@@ -204,6 +250,7 @@ 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(enable, S_IRUGO | S_IWUSR, show_enable, store_enable);
static NETCON_CLASS_ATTR(remove, S_IWUSR, NULL, store_remove);
static struct attribute *netcon_dev_attrs[] = {
@@ -215,6 +262,7 @@ static struct attribute *netcon_dev_attr
&netcon_dev_attr_remote_ip.attr,
&netcon_dev_attr_local_mac.attr,
&netcon_dev_attr_remote_mac.attr,
+ &netcon_dev_attr_enable.attr,
&netcon_dev_attr_remove.attr,
NULL
};
@@ -434,6 +482,9 @@ static void cleanup_netconsole(void)
list_for_each_entry_safe(dev, tmp, &active_netconsole_dev, list) {
kobject_unregister(&dev->obj);
}
+ list_for_each_entry_safe(dev, tmp, &inactive_netconsole_dev, list) {
+ kobject_unregister(&dev->obj);
+ }
if (netcon_miscdev_configured) {
misc_deregister(&netcon_miscdev);
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
From: Keiichi KII <[email protected]>
This patch contains the following changes.
To add port dynamically, create "add" element in /sys/class/misc/netconsole.
ex)
1. echo "eth0" > /sys/clas/misc/netconsole/add
then the port is added with the default settings.
2. echo "@/eth0,@192.168.0.1/" > /sys/class/misc/netconsole/add
then the port is added with the settings sending kernel messages
to 192.168.0.1 using eth0 device.
-+- /sys/class/misc/
|-+- netconsole/
|--- add [-w-------] If you write parameter(network interface name
| or one config parameter of netconsole), The
| port related its config is added
|--- port1/
|--- port2/
...
Signed-off-by: Keiichi KII <[email protected]>
---
--- linux-2.6.19/drivers/net/netconsole.c 2006-12-06 14:37:26.874827500 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.add 2006-12-06
13:33:05.661516750 +0900
@@ -321,6 +321,50 @@ static struct miscdevice netcon_miscdev
.name = "netconsole",
};
+static ssize_t set_netconmisc_add(struct class_device *cdev, const char *buf,
+ size_t count)
+{
+ char *target;
+ char *target_param;
+
+ target_param = (char*)kmalloc(count+1, GFP_ATOMIC);
+ if (!target_param) {
+ printk(KERN_INFO "netconsole: kmalloc() failed!\n");
+ return -ENOMEM;
+ }
+
+ strcpy(target_param, buf);
+ if (target_param[count - 1] == '\n') {
+ target_param[count - 1] = '\0';
+ }
+
+ if (dev_get_by_name(target_param)) {
+ printk(KERN_INFO "netconsole: device name = [%s]\n",
+ target_param);
+ target = (char*)kmalloc(MAX_CONFIG_LENGTH+1, GFP_ATOMIC);
+ if (!target) {
+ printk(KERN_INFO "netconsole: kmalloc() failed!\n");
+ kfree(target_param);
+ return -ENOMEM;
+ }
+ sprintf(target,"@/%s,@/", target_param);
+ add_netcon_dev(target);
+ kfree(target);
+ } else {
+ printk(KERN_INFO "netconsole: config = [%s]\n", target_param);
+ add_netcon_dev(target_param);
+ }
+ kfree(target_param);
+
+ return count;
+}
+
+static CLASS_DEVICE_ATTR(add, S_IWUSR, NULL, set_netconmisc_add);
+
+static struct class_device_attribute *netcon_misc_attr[] = {
+ &class_device_attr_add,
+};
+
static struct netpoll np = {
.name = "netconsole",
.dev_name = "eth0",
@@ -446,6 +490,7 @@ static int __init init_netconsole(void)
{
char *p;
char *tmp = config;
+ int i = 0;
if (misc_register(&netcon_miscdev)) {
printk(KERN_INFO
@@ -456,6 +501,11 @@ static int __init init_netconsole(void)
}
register_console(&netconsole);
+ for(i=0; i < ARRAY_SIZE(netcon_misc_attr); i++) {
+ class_device_create_file(netcon_miscdev.class,
+ netcon_misc_attr[i]);
+ }
+
if(!strlen(config)) {
printk(KERN_INFO "netconsole: not configured\n");
return 0;
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
From: Keiichi KII <[email protected]>
Update modification history.
Signed-off-by: Keiichi KII <[email protected]>
---
--- linux-2.6.19/drivers/net/netconsole.c 2006-12-12 14:57:45.588967500 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.sign 2006-12-12
14:54:49.541965250 +0900
@@ -15,6 +15,9 @@
* generic card hooks
* works non-modular
* 2003-09-07 rewritten with netpoll api
+ * 2006-12-12 add extended features for
+ * dynamic configurable netconsole
+ * by Keiichi KII <[email protected]>
*/
/****************************************************************
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
On Tue, Dec 12, 2006 at 03:28:17PM +0900, Keiichi KII wrote:
> From: Keiichi KII <[email protected]>
>
> This patch contains the following cleanups.
> - add __init for initialization functions(option_setup() and init_netconsole()).
> - define name of magic number.
>
> Signed-off-by: Keiichi KII <[email protected]>
> ---
> --- linux-2.6.19/drivers/net/netconsole.c 2006-12-06 14:37:06.985584500 +0900
> +++ enhanced-netconsole/drivers/net/netconsole.c.cleanup 2006-12-06
> 14:34:52.561183500 +0900
> @@ -50,8 +50,14 @@ MODULE_AUTHOR("Maintainer: Matt Mackall
> MODULE_DESCRIPTION("Console driver for network interfaces");
> MODULE_LICENSE("GPL");
>
> -static char config[256];
> -module_param_string(netconsole, config, 256, 0);
> +enum {
> + MAX_PRINT_CHUNK = 1000,
> + MAX_CONFIG_LENGTH = 256,
> +};
Converting defines to anonymous enums is generally not considered a net
improvement around here. It doesn't provide any improvement in type safety.
>
> static void write_msg(struct console *con, const char *msg, unsigned int len)
> {
> @@ -75,14 +80,12 @@ static void write_msg(struct console *co
> return;
>
> local_irq_save(flags);
> -
> for(left = len; left; ) {
> frag = min(left, MAX_PRINT_CHUNK);
> netpoll_send_udp(&np, msg, frag);
> msg += frag;
> left -= frag;
> }
> -
> local_irq_restore(flags);
> }
It's not a good idea to mix unrelated changes in, especially if
they're gratuitous formatting changes.
> +static int __init init_netconsole(void)
> {
> if(strlen(config))
> option_setup(config);
I seem to recall there was a reason for not marking that __init. It
may no longer apply.
--
Mathematics is the supreme nostalgia of our time.
On Tue, Dec 12, 2006 at 03:17:48PM +0900, Keiichi KII wrote:
> 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).
FYI, there's a robot named Dave Miller who will eventually notice your
email and scold you for not cc:ing it to netdev, claiming that he
doesn't read LKML.
--
Mathematics is the supreme nostalgia of our time.
On Tue, Dec 12, 2006 at 03:34:54PM +0900, Keiichi KII wrote:
> 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
> | |--- remove [-w-------] if you write something to "remove",
> | | this port is removed.
> | |--- dev_name [r--r--r--] network interface name
> | |--- local_ip [rw-r--r--] source IP to use, writable
> | |--- local_port [rw-r--r--] source port number for UDP packets, writable
> | |--- local_mac [r--r--r--] source MAC address
> | |--- remote_ip [rw-r--r--] port number for logging agent, writable
> | |--- remote_port [rw-r--r--] IP address for logging agent, writable
> | ---- remote_mac [rw-r--r--] MAC address for logging agent, writable
> |--- port2/
> |--- port3/
> ...
>
> Signed-off-by: Keiichi KII <[email protected]>
> ---
> --- linux-2.6.19/drivers/net/netconsole.c 2006-12-06 14:37:26.842825500 +0900
> +++ enhanced-netconsole/drivers/net/netconsole.c.sysfs 2006-12-06
> 13:32:47.488381000 +0900
> @@ -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");
> @@ -53,6 +55,7 @@ MODULE_LICENSE("GPL");
> enum {
> MAX_PRINT_CHUNK = 1000,
> MAX_CONFIG_LENGTH = 256,
> + MAC_ADDR_DIGIT = 6,
> };
>
> static char config[MAX_CONFIG_LENGTH];
> @@ -62,19 +65,214 @@ MODULE_PARM_DESC(netconsole, " netconsol
>
> struct netconsole_device {
> struct list_head list;
> + struct kobject obj;
> spinlock_t netpoll_lock;
> int id;
> struct netpoll np;
> };
>
> +struct netcon_dev_attr {
> + struct attribute attr;
> + ssize_t (*show)(struct netconsole_device*, char*);
> + ssize_t (*store)(struct netconsole_device*, const char*,
> + size_t count);
> +};
> +
> static int add_netcon_dev(const char*);
> +static void setup_netcon_dev_sysfs(struct netconsole_device*);
> static void cleanup_netconsole(void);
> static void netcon_dev_cleanup(struct netconsole_device *nd);
>
> +static int netcon_miscdev_configured = 0;
> +
> static LIST_HEAD(active_netconsole_dev);
>
> static DEFINE_SPINLOCK(netconsole_dev_list_lock);
>
> +#define SHOW_CLASS_ATTR(field, type, format, ...) \
> +static ssize_t show_##field(type, char *buf) \
> +{ \
> + return sprintf(buf, format, __VA_ARGS__); \
> +} \
I see this sort of thing is pretty common with sysfs stuff.. but yuck.
> +static ssize_t show_netcon_dev_attr(struct kobject *kobj,
> + struct attribute *attr,
> + char *buffer)
> +{
> + struct netcon_dev_attr *na = container_of(attr, struct netcon_dev_attr,
> + attr);
> + struct netconsole_device * nd =
> + container_of(kobj, struct netconsole_device, obj);
> + if (na->show) {
> + return na->show(nd, buffer);
> + } else {
> + return -EACCES;
> + }
Kernel style is to skip the braces for single statement clauses.
> + if (misc_register(&netcon_miscdev)) {
> + printk(KERN_INFO
> + "netconsole: unable netconsole misc device\n");
This error message seems to be missing a word or two.
--
Mathematics is the supreme nostalgia of our time.
On Tue, Dec 12, 2006 at 03:36:00PM +0900, Keiichi KII wrote:
> From: Keiichi KII <[email protected]>
>
> This patch contains switch function of netpoll.
>
> if "enable" attribute of certain port is '1', this port is used.
> if "enable" attribute of certain port is '0', this port isn't used.
>
> active_netconsole_dev list manages a list of active ports.
> inactive_netconsole_dev list manages a list of inactive ports.
>
> If you write '0' to "enable" attribute of a port included in
> active_netconsole_dev_list, This port moves from active_netconsole_dev to
> inactive_netconsole_dev and won't used to send kernel message.
>
> -+- /sys/class/misc/
> |-+- netconsole/
> |-+- port1/
> | |--- id [r--r--r--] id
> | |--- enable [rw-r--r--] 0: disable, 1: enable, writable
> | ...
> |--- port2/
> ...
>
> Signed-off-by: Keiichi KII <[email protected]>
> ---
> --- linux-2.6.19/drivers/net/netconsole.c 2006-12-06 14:37:26.858826500 +0900
> +++ enhanced-netconsole/drivers/net/netconsole.c.switch 2006-12-06
> 13:32:56.744959500 +0900
> @@ -86,9 +86,25 @@ static void netcon_dev_cleanup(struct ne
> static int netcon_miscdev_configured = 0;
>
> static LIST_HEAD(active_netconsole_dev);
> +static LIST_HEAD(inactive_netconsole_dev);
>
> static DEFINE_SPINLOCK(netconsole_dev_list_lock);
>
> +static ssize_t show_enable(struct netconsole_device *nd, char *buf) {
> + struct netconsole_device *dev;
> +
> + spin_lock(&netconsole_dev_list_lock);
> + list_for_each_entry(dev, &active_netconsole_dev, list) {
> + if (dev->id == nd->id) {
> + spin_unlock(&netconsole_dev_list_lock);
> + return sprintf(buf, "1\n");
> + }
> + }
> + spin_unlock(&netconsole_dev_list_lock);
> +
> + return sprintf(buf, "0\n");
> +}
If we intend to have no more than a couple dozen of these, having a
separate list makes things too complicated. Simply add an enabled
field in the struct and skip disabled targets on netconsole writes.
This function becomes 3 lines.
> +static ssize_t store_enable(struct netconsole_device *nd, const char *buf,
> + size_t count)
> +{
> + struct netconsole_device *dev, *tmp;
> + struct list_head *src, *dst;
> +
> + if (strncmp(buf, "1", 1) == 0) {
> + src = &inactive_netconsole_dev;
> + dst = &active_netconsole_dev;
> + } else if(strncmp(buf, "0", 1) == 0) {
> + src = &active_netconsole_dev;
> + dst = &inactive_netconsole_dev;
> + } else {
> + printk(KERN_INFO "netconsole: invalid argument: %s\n", buf);
> + return -EINVAL;
> + }
> +
> + spin_lock(&netconsole_dev_list_lock);
> + list_for_each_entry_safe(dev, tmp, src, list) {
> + if (dev->id == nd->id) {
> + list_del(&nd->list);
> + list_add(&nd->list, dst);
> + break;
> + }
> + }
> + spin_unlock(&netconsole_dev_list_lock);
> +
> + return count;
> +}
As does this one.
--
Mathematics is the supreme nostalgia of our time.
On Tue, Dec 12, 2006 at 03:37:20PM +0900, Keiichi KII wrote:
> From: Keiichi KII <[email protected]>
>
> This patch contains the following changes.
>
> To add port dynamically, create "add" element in /sys/class/misc/netconsole.
>
> ex)
> 1. echo "eth0" > /sys/clas/misc/netconsole/add
> then the port is added with the default settings.
What are the default settings for target IP address?
> 2. echo "@/eth0,@192.168.0.1/" > /sys/class/misc/netconsole/add
> then the port is added with the settings sending kernel messages
> to 192.168.0.1 using eth0 device.
>
> -+- /sys/class/misc/
> |-+- netconsole/
> |--- add [-w-------] If you write parameter(network interface name
> | or one config parameter of netconsole), The
> | port related its config is added
> |--- port1/
> |--- port2/
> ...
>
> Signed-off-by: Keiichi KII <[email protected]>
> ---
> --- linux-2.6.19/drivers/net/netconsole.c 2006-12-06 14:37:26.874827500 +0900
> +++ enhanced-netconsole/drivers/net/netconsole.c.add 2006-12-06
> 13:33:05.661516750 +0900
> @@ -321,6 +321,50 @@ static struct miscdevice netcon_miscdev
> .name = "netconsole",
> };
>
> +static ssize_t set_netconmisc_add(struct class_device *cdev, const char *buf,
> + size_t count)
> +{
> + char *target;
> + char *target_param;
> +
> + target_param = (char*)kmalloc(count+1, GFP_ATOMIC);
Unnecessary cast.
> + for(i=0; i < ARRAY_SIZE(netcon_misc_attr); i++) {
> + class_device_create_file(netcon_miscdev.class,
> + netcon_misc_attr[i]);
> + }
> +
This chunk looks like it goes in an earlier patch.
--
Mathematics is the supreme nostalgia of our time.
Matt Mackall wrote:
> On Tue, Dec 12, 2006 at 03:17:48PM +0900, Keiichi KII wrote:
>
> FYI, there's a robot named Dave Miller who will eventually notice your
> email and scold you for not cc:ing it to netdev, claiming that he
> doesn't read LKML.
>
Thank you for your replies and reviews.
When I reflect your reviews to my modification, I'm going to also send a set of
patches to netdev in a few days.
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
On Tue, 12 Dec 2006 15:38:42 +0900
Keiichi KII <[email protected]> wrote:
> From: Keiichi KII <[email protected]>
>
> Update modification history.
>
> Signed-off-by: Keiichi KII <[email protected]>
> ---
> --- linux-2.6.19/drivers/net/netconsole.c 2006-12-12 14:57:45.588967500 +0900
> +++ enhanced-netconsole/drivers/net/netconsole.c.sign 2006-12-12
> 14:54:49.541965250 +0900
> @@ -15,6 +15,9 @@
> * generic card hooks
> * works non-modular
> * 2003-09-07 rewritten with netpoll api
> + * 2006-12-12 add extended features for
> + * dynamic configurable netconsole
> + * by Keiichi KII <[email protected]>
> */
>
> /****************************************************************
>
We use a version control system now. The history comments in the kernel
source are considered legacy and rarely if ever changed.
If you want to see the revision history use git.
--
Stephen Hemminger <[email protected]>