Hi Urs,
i tested your patch that uses the sockets inode number instead of the pointer
address to the sock structure. It works great and has indeed a new benefit
for the debugging.
Additionally i added a counter for tx drops to the bcm that was missing in my
tests for ages:
har...@vwagwolkf320:~/linux-2.6$ cat /proc/net/can-bcm/43572
>>> socket inode 43572 / dropped 0 / dropped_tx 0 / bound vcan1 <<<
The problem of the address exposure for the callback function and its
parameter could be fixed by adding a new kernel config option - as done by
different other kernel code: CONFIG_CAN_DEBUG_NETLAYER
Unfortunately i mixed all this into one patch for this testing.
Anyway, what's your opinion about the suggested changes?
Regards,
Oliver
---
diff --git a/net/can/Kconfig b/net/can/Kconfig
index 89395b2..b57f3c4 100644
--- a/net/can/Kconfig
+++ b/net/can/Kconfig
@@ -40,5 +40,14 @@ config CAN_BCM
CAN messages are used on the bus (e.g. in automotive environments).
To use the Broadcast Manager, use AF_CAN with protocol CAN_BCM.
+config CAN_DEBUG_NETLAYER
+ bool "Enable CAN network layer debugging"
+ depends on CAN
+ default N
+ ---help---
+ Say Y here if you are developing and/or debugging protocols for the
+ CAN network layer. When enabled e.g. kernel internal addresses to
+ structures and function pointers are exposed in the procfs output
+ in /proc/net/can/rcvlist_* .
source "drivers/net/can/Kconfig"
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 08ffe9e..768e79f 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -124,8 +124,8 @@ struct bcm_sock {
struct list_head rx_ops;
struct list_head tx_ops;
unsigned long dropped_usr_msgs;
+ unsigned long dropped_tx_msgs;
struct proc_dir_entry *bcm_proc_read;
- char procname [9]; /* pointer printed in ASCII with \0 */
};
static inline struct bcm_sock *bcm_sk(const struct sock *sk)
@@ -165,10 +165,9 @@ static int bcm_proc_show(struct seq_file *m, void *v)
struct bcm_sock *bo = bcm_sk(sk);
struct bcm_op *op;
- seq_printf(m, ">>> socket %p", sk->sk_socket);
- seq_printf(m, " / sk %p", sk);
- seq_printf(m, " / bo %p", bo);
+ seq_printf(m, ">>> socket inode %lu", sock_i_ino(sk));
seq_printf(m, " / dropped %lu", bo->dropped_usr_msgs);
+ seq_printf(m, " / dropped_tx %lu", bo->dropped_tx_msgs);
seq_printf(m, " / bound %s", bcm_proc_getifname(ifname, bo->ifindex));
seq_printf(m, " <<<\n");
@@ -266,7 +265,12 @@ static void bcm_can_tx(struct bcm_op *op)
/* send with loopback */
skb->dev = dev;
skb->sk = op->sk;
- can_send(skb, 1);
+
+ if (can_send(skb, 1)) {
+ struct bcm_sock *bo = bcm_sk(op->sk);
+
+ bo->dropped_tx_msgs++;
+ }
/* update statistics */
op->currframe++;
@@ -1426,6 +1430,7 @@ static int bcm_release(struct socket *sock)
struct sock *sk = sock->sk;
struct bcm_sock *bo = bcm_sk(sk);
struct bcm_op *op, *next;
+ char proc_fname[32];
/* remove bcm_ops, timer, rx_unregister(), etc. */
@@ -1465,8 +1470,10 @@ static int bcm_release(struct socket *sock)
}
/* remove procfs entry */
- if (proc_dir && bo->bcm_proc_read)
- remove_proc_entry(bo->procname, proc_dir);
+ if (proc_dir && bo->bcm_proc_read) {
+ snprintf(proc_fname, sizeof(proc_fname), "%lu", sock_i_ino(sk));
+ remove_proc_entry(proc_fname, proc_dir);
+ }
/* remove device reference */
if (bo->bound) {
@@ -1489,6 +1496,7 @@ static int bcm_connect(struct socket *sock, struct
sockaddr *uaddr, int len,
struct sockaddr_can *addr = (struct sockaddr_can *)uaddr;
struct sock *sk = sock->sk;
struct bcm_sock *bo = bcm_sk(sk);
+ char proc_fname[32];
if (len < sizeof(*addr))
return -EINVAL;
@@ -1521,8 +1529,8 @@ static int bcm_connect(struct socket *sock, struct
sockaddr *uaddr, int len,
if (proc_dir) {
/* unique socket address as filename */
- sprintf(bo->procname, "%p", sock);
- bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
+ snprintf(proc_fname, sizeof(proc_fname), "%lu", sock_i_ino(sk));
+ bo->bcm_proc_read = proc_create_data(proc_fname, 0644,
proc_dir,
&bcm_proc_fops, sk);
}
diff --git a/net/can/proc.c b/net/can/proc.c
index f4265cc..6011547 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -208,8 +208,12 @@ static void can_print_rcvlist(struct seq_file *m, struct
hlist_head *rx_list,
" %-5s %03X %08x %08lx %08lx %8ld %s\n";
seq_printf(m, fmt, DNAME(dev), r->can_id, r->mask,
- (unsigned long)r->func, (unsigned long)r->data,
- r->matches, r->ident);
+#ifdef CONFIG_CAN_DEBUG_NETLAYER
+ (unsigned long)r->func, (unsigned long)r->data,
+#else
+ 0, 0,
+#endif
+ r->matches, r->ident);
}
}
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
