This patch adds the CAN core functionality but no protocols or drivers.
No protocol implementations are included here. They come as separate
patches. Protocol numbers are already in include/linux/can.h.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by: Urs Thuermann [EMAIL
David Miller [EMAIL PROTECTED] writes:
I really frown upon these local debugging macros people tend to want
to submit with their changes.
It really craps up the tree, even though it might be useful to you.
We have now removed the debug code completely. The code is quite
stable and we won't
From: Urs Thuermann [EMAIL PROTECTED]
Date: 16 Nov 2007 15:33:08 +0100
I am not aware of any useful kernel facilities to replace our debug
macros, i.e. printing of debug messages, controlled by a bit mask
passed in as a module parameter, hexdumping of fames, etc. Of course,
there are other
On Thu, 2007-11-15 at 08:40 +0100, Oliver Hartkopp wrote:
Stephen Hemminger wrote:
+#ifdef CONFIG_CAN_DEBUG_CORE
+extern void can_debug_skb(struct sk_buff *skb);
+extern void can_debug_cframe(const char *msg, struct can_frame *cframe);
+#define DBG(fmt, args...) (DBG_VAR 1 ? printk( \
Stephen Hemminger [EMAIL PROTECTED] writes:
+#ifdef CONFIG_CAN_DEBUG_CORE
+extern void can_debug_skb(struct sk_buff *skb);
+extern void can_debug_cframe(const char *msg, struct can_frame *cframe);
+#define DBG(fmt, args...) (DBG_VAR 1 ? printk( \
+
Joe Perches [EMAIL PROTECTED] writes:
On Thu, 2007-11-15 at 08:40 +0100, Oliver Hartkopp wrote:
Stephen Hemminger wrote:
+#ifdef CONFIG_CAN_DEBUG_CORE
+extern void can_debug_skb(struct sk_buff *skb);
+extern void can_debug_cframe(const char *msg, struct can_frame *cframe);
+#define
From: Urs Thuermann [EMAIL PROTECTED]
Date: 15 Nov 2007 12:51:34 +0100
I prefer our code because it is shorter (fits into one line) and can
be used anywhere where an expression is allowed compared to only where
a statement is allowed. Actually, I first had
#define DBG( ... )
+
+struct timer_list stattimer; /* timer for statistics update */
+struct s_stats stats; /* packet statistics */
+struct s_pstats pstats; /* receive list statistics */
More global variables without prefix.
These variables are not exported with EXPORT_SYMBOL, so
On Thu, Nov 15, 2007 at 04:05:30AM -0800, David Miller wrote:
From: Urs Thuermann [EMAIL PROTECTED]
Date: 15 Nov 2007 12:51:34 +0100
I prefer our code because it is shorter (fits into one line) and can
be used anywhere where an expression is allowed compared to only where
a statement is
On Wed, 14 Nov 2007 13:13:41 +0100
Urs Thuermann [EMAIL PROTECTED] wrote:
This patch adds the CAN core functionality but no protocols or drivers.
No protocol implementations are included here. They come as separate
patches. Protocol numbers are already in include/linux/can.h.
Stephen Hemminger wrote:
+#ifdef CONFIG_CAN_DEBUG_CORE
+extern void can_debug_skb(struct sk_buff *skb);
+extern void can_debug_cframe(const char *msg, struct can_frame *cframe);
+#define DBG(fmt, args...) (DBG_VAR 1 ? printk( \
+KERN_DEBUG DBG_PREFIX :
Arnaldo Carvalho de Melo [EMAIL PROTECTED] writes:
+struct sockaddr_can {
+ sa_family_t can_family;
+ int can_ifindex;
+ union {
+ struct { canid_t rx_id, tx_id; } tp16;
+ struct { canid_t rx_id, tx_id; } tp20;
+ struct { canid_t rx_id,
Em Thu, Oct 04, 2007 at 01:51:47PM +0200, Urs Thuermann escreveu:
Arnaldo Carvalho de Melo [EMAIL PROTECTED] writes:
+struct sockaddr_can {
+ sa_family_t can_family;
+ int can_ifindex;
+ union {
+ struct { canid_t rx_id, tx_id; } tp16;
+ struct {
Em Tue, Oct 02, 2007 at 03:10:08PM +0200, Urs Thuermann escreveu:
This patch adds the CAN core functionality but no protocols or drivers.
No protocol implementations are included here. They come as separate
patches. Protocol numbers are already in include/linux/can.h.
Signed-off-by: Oliver
On Tue, 2007-09-25 at 14:09 -0700, David Miller wrote:
Then please make all exported symbols marked EXPORT_SYMBOL_GPL to make
sure that the other CAN protocol can not reuse your infrastructure.
We don't want to force other CAN protocol implementations to be GPL
also. AFAIR from
From: Thomas Gleixner [EMAIL PROTECTED]
Date: Fri, 28 Sep 2007 18:27:19 +0200
I'm not inclined either way and we really should not make this a
religious question whether that code goes in or not, especially not when
we granted the mac80211 to export everything w/o _GPL suffix not too
long
On Fri, 2007-09-28 at 13:20 -0700, David Miller wrote:
That's not true with CAN.
With this CAN stuff, any driver you write for it is intimately
integrated into the design and architecture of the CAN subsystem. Any
such driver cannot stand on it's own. Look at how these drivers can
get
Em Tue, Sep 25, 2007 at 02:20:31PM +0200, Urs Thuermann escreveu:
+
+static void can_sock_destruct(struct sock *sk)
+{
+ DBG(called for sock %p\n, sk);
+
+ skb_queue_purge(sk-sk_receive_queue);
+ if (sk-sk_protinfo)
+ kfree(sk-sk_protinfo);
+}
Is it really needed
Arnaldo Carvalho de Melo [EMAIL PROTECTED] writes:
+ skb_queue_purge(sk-sk_receive_queue);
+ if (sk-sk_protinfo)
+ kfree(sk-sk_protinfo);
+}
Is it really needed to do this sk_protinfo check?
Thanks for finding this. This is from 2.6.12 times or so. We have
other CAN
On 25 Sep 2007 15:24:33 +0200
Urs Thuermann [EMAIL PROTECTED] wrote:
Arnaldo Carvalho de Melo [EMAIL PROTECTED] writes:
+ skb_queue_purge(sk-sk_receive_queue);
+ if (sk-sk_protinfo)
+ kfree(sk-sk_protinfo);
+}
Is it really needed to do this sk_protinfo check?
Thanks
Stephen Hemminger [EMAIL PROTECTED] writes:
Then please make all exported symbols marked EXPORT_SYMBOL_GPL to make
sure that the other CAN protocol can not reuse your infrastructure.
We don't want to force other CAN protocol implementations to be GPL
also. AFAIR from discussions on LKML, it
On 25 Sep 2007 23:00:15 +0200
Urs Thuermann [EMAIL PROTECTED] wrote:
Stephen Hemminger [EMAIL PROTECTED] writes:
Then please make all exported symbols marked EXPORT_SYMBOL_GPL to
make sure that the other CAN protocol can not reuse your
infrastructure.
We don't want to force other CAN
From: Urs Thuermann [EMAIL PROTECTED]
Date: 25 Sep 2007 23:00:15 +0200
Stephen Hemminger [EMAIL PROTECTED] writes:
Then please make all exported symbols marked EXPORT_SYMBOL_GPL to make
sure that the other CAN protocol can not reuse your infrastructure.
We don't want to force other CAN
Joe Perches [EMAIL PROTECTED] writes:
I'd prefer something like this, which removes the unnecessary
kmalloc/kfree pairs or the equivalent conversions to functions.
I have changed this to a static buffer. Since this is only in
#ifdef CONFIG_CAN_DEBUG_CORE, it shouldn't hurt.
#define
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
You drop the module reference again when leaving this function.
So sock-ops might contain a stale pointer if the module is
unloaded after this. You need to either keep the module reference
while the socket is alive or remove stale
Joe Perches [EMAIL PROTECTED] writes:
On Thu, 2007-09-20 at 20:43 +0200, Urs Thuermann wrote:
+#define DBG(...) (debug 1 ? \
+ (printk(KERN_DEBUG can-%s %s: , \
+ IDENT, __func__), printk(args)) : 0)
+#define
Urs Thuermann wrote:
+static int can_create(struct net *net, struct socket *sock, int protocol)
+{
+ ...
+
+ spin_lock(proto_tab_lock);
+ cp = proto_tab[protocol];
+ if (cp !try_module_get(cp-prot-owner))
+ cp = NULL;
+ spin_unlock(proto_tab_lock);
+
+
On Fri, 2007-09-21 at 12:35 +0200, Urs Thuermann wrote:
I didn't find a way with gcc-2.95 to make the format
string a separate macro argument (which I also wanted).
The old 2.x GCC workaround was to use
#define DBG(fmt, arg) printk(fmt , ## arg)
adding a space before the last comma.
I
Patrick McHardy [EMAIL PROTECTED] writes:
You drop the module reference again when leaving this function.
So sock-ops might contain a stale pointer if the module is
unloaded after this. You need to either keep the module reference
while the socket is alive or remove stale references when
Hi Patrick,
I have done allmost all changes to the code as you suggested. The
changes to use the return value of can_rx_register() also fixed a
minor flax with failing bind() and setsockopt() on raw sockets.
But there are two things left I would like to ask/understand:
Patrick McHardy [EMAIL
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
When the module is unloaded it calls can_proto_unregister() which
clears the pointer. Do you see a race condition here?
Yes, you do request_module, load the module, get the cp pointer
from proto_tab, the module is unloaded again.
Patrick McHardy [EMAIL PROTECTED] writes:
No, you need to add your own locking to prevent this, something
list this:
registration/unregistration:
take lock
change proto_tab[]
release lock
lookup:
take lock
cp = proto_tab[]
if (cp !try_module_get(cp-owner))
cp = NULL
On Thu, 2007-09-20 at 20:43 +0200, Urs Thuermann wrote:
+#define DBG(...) (debug 1 ? \
+ (printk(KERN_DEBUG can-%s %s: , \
+ IDENT, __func__), printk(args)) : 0)
+#define DBG_FRAME(args...) (debug 2 ? can_debug_cframe(args) :
On Thu, 2007-09-20 at 13:06 -0700, Joe Perches wrote:
On Thu, 2007-09-20 at 20:43 +0200, Urs Thuermann wrote:
+#define DBG(...) (debug 1 ? \
+ (printk(KERN_DEBUG can-%s %s: , \
+ IDENT, __func__), printk(args)) : 0)
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
+HLIST_HEAD(rx_dev_list);
Same here (static).
Can't be static since it's used in proc.c. But __read_mostly might
make sense.
What exactly is the effect of __read_mostly? Is that in a separate
ELF section? Where is
Urs Thuermann wrote:
This patch adds the CAN core functionality but no protocols or drivers.
No protocol implementations are included here. They come as separate
patches. Protocol numbers are already in include/linux/can.h.
Signed-off-by: Oliver Hartkopp [EMAIL PROTECTED]
Signed-off-by:
Urs Thuermann wrote:
Patrick McHardy [EMAIL PROTECTED] writes:
Looks pretty good, please see below for a few comments (mostly minor
nitpicking, a few things that look like real bugs). Nothing that
couldn't be fixed after merging though.
Thank you for your review. I'll go through it and
Patrick McHardy [EMAIL PROTECTED] writes:
Looks pretty good, please see below for a few comments (mostly minor
nitpicking, a few things that look like real bugs). Nothing that
couldn't be fixed after merging though.
Thank you for your review. I'll go through it and your other mail
this
Patrick McHardy [EMAIL PROTECTED] writes:
+++ net-2.6.24/include/linux/can.h 2007-09-17 10:27:09.0 +0200
Is this file used only from within the kernel? If so you could use
the nicer-to-look-at u8/u16/u32 types instead of the double underscored
ones.
No, this file contains the
Hi Urs, Hello Paul,
i assume Paul refers to the can_rx_delete_all() function that adds each
receive list entry for rcu removal using the can_rx_delete RCU callback,
right?
So the idea would be to create a second RCU callback - e.g.
can_rx_delete_list() - that removes the complete list
On Fri, May 18, 2007 at 11:19:01AM +0200, Oliver Hartkopp wrote:
Hi Urs, Hello Paul,
i assume Paul refers to the can_rx_delete_all() function that adds each
receive list entry for rcu removal using the can_rx_delete RCU callback,
right?
So the idea would be to create a second RCU
Hm, this is indeed one step further, than i thought :-)
Thanks for this nifty solution!
I will doublecheck your suggestion with Urs and then we'll change it in
our next patch update (after some more feedback on this mailing list).
Additional feedback is welcome.
Tnx best regards,
Oliver
Hello Paul,
as you may see in the attached SVN-log i changed some code according
your suggestions.
I additionally made some clarifications of function names.
If you would like to see the af_can.c completely please visit:
On 5/16/07, Urs Thuermann [EMAIL PROTECTED] wrote:
This patch adds the CAN core functionality but no protocols or drivers.
No protocol implementations are included here. They come as separate
patches. Protocol numbers are already in include/linux/can.h.
Signed-Off-By: Oliver Hartkopp [EMAIL
Arnaldo Carvalho de Melo wrote:
SNIP
+
+/**
+ * struct sockaddr_can - the sockaddr structure for CAN sockets
+ * @can_family: address family number AF_CAN.
+ * @can_ifindex: CAN network interface index.
+ * @can_addr:transport protocol specific address, mostly CAN IDs.
+ */
On 16 May 2007 21:14:20 +0200, Urs Thuermann [EMAIL PROTECTED] wrote:
Arnaldo Carvalho de Melo [EMAIL PROTECTED] writes:
Can can_ifindex be turned into a unsigned short? That way we would
have it nicely packed, avoiding this hole:
Since dev-ifindex is int and we have many assignments between
46 matches
Mail list logo
