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

2007-01-10 Thread Keiichi KII
 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.
   
 IMHO this kind of magic side effect is a misuse of sysfs. and would
 make proper locking
 impossible. How do you deal with the dangling reference to the
 netconsole object?

I manage the reference by using a list.
If you write something to remove, 
firstly the port entry is removed from sysfs and then
the reference is removed from the list and a resource of the port is freed.

 f= open (... netconsole/port1/remove)
 write(f, , 1)
 sleep(2)
 write(f, , 1)  this probably would crash...
 
 
 Maybe having a state variable/sysfs file so you could setup the port and
 turn it on/off with write.

You are right.
When I tested above program, my machine crashed.

I'm going to rethink the interface for netconsole.

Thanks for your comments.
-- 
Keiichi KII
NEC Corporation OSS Promotion Center
E-mail: [EMAIL PROTECTED]



-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2006-12-29 Thread Stephen Hemminger
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.
   
IMHO this kind of magic side effect is a misuse of sysfs. and would
make proper locking
impossible. How do you deal with the dangling reference to the
netconsole object?
f= open (... netconsole/port1/remove)
write(f, , 1)
sleep(2)
write(f, , 1)  this probably would crash...


Maybe having a state variable/sysfs file so you could setup the port and
turn it on/off with write.
| |--- dev_name[r--r--r--]  network interface name
   

Please don't use dev_name, instead use a a symlink. You see if the
device is renamed,
the dev_name will be wrong, but the symlink to the net_device kobject
should be okay.
| |--- 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]
 Signed-off-by: Takayoshi Kochi [EMAIL PROTECTED]
   

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC][PATCH -mm take2 3/5] add interface for netconsole using sysfs

2006-12-25 Thread 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
   | |--- 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]
Signed-off-by: Takayoshi Kochi [EMAIL PROTECTED]
---
[changes]
1. use ETH_ALEN from if_ether.h.
2. remove initialization for static variable.
3. follow the kernel coding style.

--- linux-mm/drivers/net/netconsole.c   2006-12-26 14:19:01.441854500 +0900
+++ enhanced-netconsole/drivers/net/netconsole.c.sysfs  2006-12-25 
16:44:49.520912750 +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,231 @@ 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 ;
+
+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;
+
 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,
+