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.20-rc1-mm1 and is divided to each function.
Your comments are very welcome.
Signed-off-by: Keiichi KII <[email protected]>
---
[changes]
1. change kernel base from 2.6.19 to 2.6.20-rc1-mm1.
--
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]>
---
[changes]
1. expand macro code as far as possible.
2. follow kernel coding style.
3. print proper output messeage.
4. attach proper label for printk.
5. integrate netpoll_lock and netcon_target_list_lock into common spinlock.
6. return proper error value.
--- linux-mm/drivers/net/netconsole.c 2006-12-22 20:54:54.431673500 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.sysfs 2006-12-22 16:12:47.925833000 +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");
@@ -56,18 +58,234 @@ MODULE_PARM_DESC(netconsole, " netconsol
struct netconsole_target {
struct list_head list;
+ struct kobject obj;
int id;
struct netpoll np;
};
+#define MAX_PRINT_CHUNK 1000
+#define CONFIG_SEPARATOR ":"
+#define MAC_ADDR_DIGIT 6
+
+struct target_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct netconsole_target*, char*);
+ ssize_t (*store)(struct netconsole_target*, const char*,
+ size_t count);
+};
+
static int add_target(char* target_config);
+static void setup_target_sysfs(struct netconsole_target *nt);
static void cleanup_netconsole(void);
static void delete_target(struct netconsole_target *nt);
+static int miscdev_configured = 0;
+
static LIST_HEAD(target_list);
static DEFINE_SPINLOCK(target_list_lock);
+static ssize_t show_id(struct netconsole_target *nt, char *buf)
+{
+ return sprintf(buf, "%d\n", nt->id);
+}
+
+static ssize_t show_dev_name(struct netconsole_target *nt, char *buf)
+{
+ return sprintf(buf, "%s\n", nt->np.dev_name);
+}
+
+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[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(&target_list_lock);
+ memcpy(nt->np.remote_mac, input_mac, MAC_ADDR_DIGIT);
+ spin_unlock(&target_list_lock);
+
+ return count;
+}
+
+static ssize_t store_remove(struct netconsole_target *nt, const char *buf,
+ size_t count)
+{
+ kobject_unregister(&nt->obj);
+ return 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(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 *target_attrs[] = {
+ &target_attr_id.attr,
+ &target_attr_dev_name.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,
+ &target_attr_remove.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 -EACCES;
+}
+
+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 -EACCES;
+}
+
+static void release_target(struct kobject *kobj)
+{
+ struct netconsole_target *nt =
+ container_of(kobj, struct netconsole_target, obj);
+
+ delete_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 struct netpoll np = {
.name = "netconsole",
.dev_name = "eth0",
@@ -76,9 +294,6 @@ static struct netpoll np = {
.remote_mac = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
};
-#define MAX_PRINT_CHUNK 1000
-#define CONFIG_SEPARATOR ":"
-
static int add_target(char* target_config)
{
int retval = 0;
@@ -119,10 +334,19 @@ static int add_target(char* target_confi
list_add(&new_target->list, &target_list);
spin_unlock(&target_list_lock);
+ setup_target_sysfs(new_target);
out:
return retval;
}
+static void setup_target_sysfs(struct netconsole_target *nt)
+{
+ kobject_set_name(&nt->obj, "port%d", nt->id);
+ nt->obj.parent = &netconsole_miscdev.class->kobj;
+ nt->obj.ktype = &target_ktype;
+ kobject_register(&nt->obj);
+}
+
static void delete_target(struct netconsole_target *nt)
{
spin_lock(&target_list_lock);
@@ -178,6 +402,15 @@ static int __init init_netconsole(void)
char *p;
char *tmp = config;
+ 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)) {
@@ -202,9 +435,13 @@ static void cleanup_netconsole(void)
struct netconsole_target *nt, *tmp;
unregister_console(&netconsole);
+
list_for_each_entry_safe(nt, tmp, &target_list, list) {
- delete_target(nt);
+ kobject_unregister(&nt->obj);
}
+
+ if (miscdev_configured)
+ misc_deregister(&netconsole_miscdev);
}
module_init(init_netconsole);
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
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/
...
Signed-off-by: Keiichi KII <[email protected]>
---
[changes]
1. change active/inactive netpoll from list management to flag management.
2. change the name of switch from "enable" to "enabled".
3. return proper error value.
--- linux-mm/drivers/net/netconsole.c 2006-12-22 20:54:54.495677500 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.switch 2006-12-22 16:12:54.746259250 +0900
@@ -60,6 +60,7 @@ struct netconsole_target {
struct list_head list;
struct kobject obj;
int id;
+ int enabled;
struct netpoll np;
};
@@ -131,10 +132,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);
@@ -145,6 +155,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);
@@ -155,6 +169,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);
@@ -165,6 +183,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);
@@ -189,12 +211,39 @@ static ssize_t store_remote_mac(struct n
cur = delim + 1;
}
spin_lock(&target_list_lock);
+ if (nt->enabled) {
+ spin_unlock(&target_list_lock);
+ return -EINVAL;
+ }
memcpy(nt->np.remote_mac, input_mac, MAC_ADDR_DIGIT);
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 (strncmp(buf, "1", 1) == 0) {
+ enabled = 1;
+ } else if(strncmp(buf, "0", 1) == 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;
+}
+
static ssize_t store_remove(struct netconsole_target *nt, const char *buf,
size_t count)
{
@@ -219,6 +268,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 NETCON_CLASS_ATTR(remove, S_IWUSR, NULL, store_remove);
static struct attribute *target_attrs[] = {
@@ -230,6 +281,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,
&target_attr_remove.attr,
NULL
};
@@ -245,7 +297,7 @@ static ssize_t show_target_attr(struct k
if (na->show)
return na->show(nt, buffer);
else
- return -EACCES;
+ return -EINVAL;
}
static ssize_t store_target_attr(struct kobject *kobj,
@@ -259,7 +311,7 @@ static ssize_t store_target_attr(struct
if (na->store)
return na->store(nt, buffer, count);
else
- return -EACCES;
+ return -EINVAL;
}
static void release_target(struct kobject *kobj)
@@ -325,6 +377,7 @@ static int add_target(char* target_confi
atomic_inc(&target_count);
new_target->id = atomic_read(&target_count);
+ new_target->enabled = 1;
printk(KERN_INFO "netconsole: add target: "
"remote ip_addr=%d.%d.%d.%d remote port=%d\n",
@@ -371,7 +424,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;
--
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)
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]>
---
[changes]
1. remove unneccesary cast.
2. change config parameter for "add" element.
--- linux-mm/drivers/net/netconsole.c 2006-12-22 20:54:54.531679750 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.add 2006-12-22 16:13:02.062716500 +0900
@@ -338,6 +338,30 @@ static struct miscdevice netconsole_misc
.name = "netconsole",
};
+static ssize_t store_miscdev_add(struct class_device *cdev,
+ const char *buf, size_t count)
+{
+ char *target_param;
+
+ target_param = kmalloc(count+1, GFP_KERNEL);
+ if (!target_param) {
+ printk(KERN_ERR "netconsole: kmalloc() failed!\n");
+ return -ENOMEM;
+ }
+
+ strcpy(target_param, buf);
+ if (target_param[count - 1] == '\n')
+ target_param[count - 1] = '\0';
+
+ printk(KERN_INFO "netconsole: config = [%s]\n", target_param);
+ add_target(target_param);
+ kfree(target_param);
+
+ return count;
+}
+
+static CLASS_DEVICE_ATTR(add, S_IWUSR, NULL, store_miscdev_add);
+
static struct netpoll np = {
.name = "netconsole",
.dev_name = "eth0",
@@ -467,6 +491,9 @@ static int __init init_netconsole(void)
register_console(&netconsole);
+ class_device_create_file(netconsole_miscdev.class,
+ &class_device_attr_add);
+
if(!strlen(config)) {
printk(KERN_ERR "netconsole: not configured\n");
return 0;
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
On Fri, 22 Dec 2006 21:01:09 +0900
Keiichi KII <[email protected]> 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).
>
> 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.
If netconsole is a module, you should be able to remove it and reload
with different parameters.
> 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.20-rc1-mm1 and is divided to each function.
> Your comments are very welcome.
Rather than extending the existing kludge with module parameter, to
sysfs. I would rather see a better API for this. Please build think
about doing a better API with a basic set of ioctl's. Some additional
things:
- shouldn't just be IPV4 specific, should handle IPV6 as well
- shouldn't specify MAC address, it can do network discovery/arp to
find that when adding addresses
On Fri, 22 Dec 2006 21:14:36 +0900 Keiichi KII wrote:
> From: Keiichi KII <[email protected]>
>
> ---
> [changes]
> 1. expand macro code as far as possible.
> 2. follow kernel coding style.
> 3. print proper output messeage.
> 4. attach proper label for printk.
> 5. integrate netpoll_lock and netcon_target_list_lock into common spinlock.
> 6. return proper error value.
>
> --- linux-mm/drivers/net/netconsole.c 2006-12-22 20:54:54.431673500 +0900
> +++ enhanced-netconsole/drivers/net/netconsole.c.sysfs 2006-12-22 16:12:47.925833000 +0900
> @@ -56,18 +58,234 @@ MODULE_PARM_DESC(netconsole, " netconsol
>
> struct netconsole_target {
> struct list_head list;
> + struct kobject obj;
> int id;
> struct netpoll np;
> };
>
> +#define MAX_PRINT_CHUNK 1000
> +#define CONFIG_SEPARATOR ":"
> +#define MAC_ADDR_DIGIT 6
Can you use ETH_ALEN from if_ether.h instead of MAC_ADDR_DIGIT ?
> +
> +struct target_attr {
> + struct attribute attr;
> + ssize_t (*show)(struct netconsole_target*, char*);
> + ssize_t (*store)(struct netconsole_target*, const char*,
> + size_t count);
> +};
> +
> static int add_target(char* target_config);
> +static void setup_target_sysfs(struct netconsole_target *nt);
> static void cleanup_netconsole(void);
> static void delete_target(struct netconsole_target *nt);
>
> +static int miscdev_configured = 0;
Don't init static data to 0 or NULL.
It's done for us.
> static LIST_HEAD(target_list);
>
> static DEFINE_SPINLOCK(target_list_lock);
>
> +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));
I don't understand the use of HIPQUAD() here instead of
NIPQUAD(). Explain?
Also, NIPQUAD_FMT (in kernel.h) uses "%u.%u.%u.%u".
This should probably be the same.
Or just use: NIPQUAD_FMT "\n"
> +}
> +
> +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 store_remote_mac(struct netconsole_target *nt, 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++) {
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;
No braces on one-line "blocks", please.
> + }
> + cur = delim + 1;
> + }
> + spin_lock(&target_list_lock);
> + memcpy(nt->np.remote_mac, input_mac, MAC_ADDR_DIGIT);
> + spin_unlock(&target_list_lock);
> +
> + return count;
> +}
> +
> +
---
~Randy
Thank you for your comments.
>> 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.20-rc1-mm1 and is divided to each function.
>> Your comments are very welcome.
>
> Rather than extending the existing kludge with module parameter, to
> sysfs. I would rather see a better API for this. Please build think
> about doing a better API with a basic set of ioctl's. Some additional
What advantage do we use a set of ioctl's compared to sysfs?
I think that sysfs is easier and more readable than the ioctl's
to change configurations(IP address and port number and so on).
ex)
# cat /sys/class/misc/netconsole/port1/remote_ip
192.168.0.1
# echo 172.16.0.1 > /sys/class/misc/netconsole/port1/remote_ip
# cat /sys/class/misc/netconsole/port1/remote_ip
172.16.0.1
And the sysfs doesn't need to create access program such as the ioctl's.
If you change configurations related to netconsole through the sysfs interface,
a simple script file including a set of commands such as above echo
will help you set up automatically.
> things:
> - shouldn't just be IPV4 specific, should handle IPV6 as well
I would like to implement handling IPV6 on demand in the future.
> - shouldn't specify MAC address, it can do network discovery/arp to
> find that when adding addresses
I think a userland application would rather find target MAC address and
change it through the sysfs.
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]
Thank you for your replies and reviews.
I will follow your advices.
>> static LIST_HEAD(target_list);
>>
>> static DEFINE_SPINLOCK(target_list_lock);
>>
>> +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));
>
> I don't understand the use of HIPQUAD() here instead of
> NIPQUAD(). Explain?
>
> Also, NIPQUAD_FMT (in kernel.h) uses "%u.%u.%u.%u".
> This should probably be the same.
> Or just use: NIPQUAD_FMT "\n"
IP address is stored in the form of host byte order in netpoll structure.
So, You can't use NIPQUAD to follow the current implementation of netpoll.
--
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [email protected]