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 ot
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
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 sta
> > > +
> > > +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 EXPOR
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( ... )
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 *cfra
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( \
> > +
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
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_P
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.
>
> Signed
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;
> > > +
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 { ca
Arnaldo Carvalho de Melo wrote:
Em Tue, Oct 02, 2007 at 03:10:08PM +0200, Urs Thuermann escreveu:
+
+/**
+ * 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
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: Ol
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
> ge
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
> lon
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: 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
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 for
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, i
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_
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 ha
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
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 can_
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
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 whe
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.
>
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_
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
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)
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(ar
Urs Thuermann wrote:
> I will post our updated code again, probably today. The issues still
> left are
>
> * module parameter for loopback, but we want to keep that.
No objections.
> * configure option for allowing normal users access to raw and bcm CAN
> sockets. I'll check how easily an (e
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))
>
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 u
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 P
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 sec
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 th
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 th
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 e
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-of
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:
http://svn.berlios.de/svnroot/repos/socketcan/trunk/kernel/2.6/net/can/af
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
Pa
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
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 inside
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
"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
can_ifindex and dev->ifindex it would not make sense to define
ca
Arnaldo Carvalho de Melo wrote:
> +
> +/**
> + * 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.
> + */
> +st
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
48 matches
Mail list logo
