* Re: [PATCH net V2 2/2] virtio-net: refill only when device is up during setting queues
From: David Miller @ 2013-10-17 19:55 UTC (permalink / raw)
To: jasowang; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <1381807139-3450-2-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Tue, 15 Oct 2013 11:18:59 +0800
> We used to schedule the refill work unconditionally after changing the
> number of queues. This may lead an issue if the device is not
> up. Since we only try to cancel the work in ndo_stop(), this may cause
> the refill work still work after removing the device. Fix this by only
> schedule the work when device is up.
>
> The bug were introduce by commit 9b9cd8024a2882e896c65222aa421d461354e3f2.
> (virtio-net: fix the race between channels setting and refill)
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable, thanks Jason.
^ permalink raw reply
* Re: [PATCH -next] netdev: inet_timewait_sock.h missing semi-colon when KMEMCHECK is enabled
From: David Miller @ 2013-10-17 19:57 UTC (permalink / raw)
To: rdunlap; +Cc: thierry.reding, linux-next, linux-kernel, broonie, netdev
In-Reply-To: <525C47C0.2000907@infradead.org>
From: Randy Dunlap <rdunlap@infradead.org>
Date: Mon, 14 Oct 2013 12:36:32 -0700
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fix (a few hundred) build errors due to missing semi-colon when
> KMEMCHECK is enabled:
>
> include/net/inet_timewait_sock.h:139:2: error: expected ',', ';' or '}' before 'int'
> include/net/inet_timewait_sock.h:148:28: error: 'const struct inet_timewait_sock' has no member named 'tw_death_node'
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Applied, thanks Randy.
^ permalink raw reply
* Re: [net-next REPOST] 8390 ei_debug : Reenable the use of debugging in 8390 based chips
From: David Miller @ 2013-10-17 19:59 UTC (permalink / raw)
To: tedheadster; +Cc: netdev
In-Reply-To: <1381805161-18833-1-git-send-email-tedheadster@gmail.com>
From: Matthew Whitehead <tedheadster@gmail.com>
Date: Mon, 14 Oct 2013 22:46:01 -0400
> Ethernet boards based on the 8390 chip had an '#ifdef notdef' disabling the
> use of the debug variable ei_debug. Reenable it for those of us who still
> occasionally use it.
>
> Also handle the case of the 'ne' driver which uses 8390p.o rather than
> 8390.o. In that case ei_debug is aliased to eip_debug so it doesn't clash
> with the previously exported ei_debug.
>
> Signed-off-by: Matthew Whitehead <tedheadster@gmail.com>
I'd rather not encourage the use of things like this.
We have a standard, run time, way to enable and disable debugging
output on a per-netdevice basis. And the "default" value of which can
be controlled by a module parameter if need be.
Please convert the driver to this method instead.
Thank you.
^ permalink raw reply
* Re: [PATCH] net: qmi_wwan: Olivetti Olicard 200 support
From: David Miller @ 2013-10-17 20:01 UTC (permalink / raw)
To: mrkiko.rs
Cc: gregkh, bjorn, dcbw, christian.schmiedl, linux-usb, netdev,
linux-kernel, anto.pellizzari83
In-Reply-To: <1381842408-10800-2-git-send-email-mrkiko.rs@gmail.com>
From: Enrico Mioso <mrkiko.rs@gmail.com>
Date: Tue, 15 Oct 2013 15:06:48 +0200
> This is a QMI device, manufactured by TCT Mobile Phones.
> A companion patch blacklisting this device's QMI interface in the option.c
> driver has been sent.
>
> Signed-off-by: Enrico Mioso <mrkiko.rs@gmail.com>
> Signed-off-by: Antonella Pellizzari <anto.pellizzari83@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] X.25: Fix address field length calculation
From: David Miller @ 2013-10-17 20:04 UTC (permalink / raw)
To: GKelleter; +Cc: andrew.hendry, linux-x25, netdev, linux-kernel
In-Reply-To: <525D5131.9070007@datus.com>
From: Kelleter, Günther <GKelleter@datus.com>
Date: Tue, 15 Oct 2013 14:29:06 +0000
> Addresses are BCD encoded, not ASCII. x25_addr_ntoa got it right.
>
> Signed-off-by: Guenther Kelleter <gkelleter@datus.com>
> ---
> Wrong length calculation leads to rejection of CALL ACCEPT packets.
This patch doesn't apply because it was severely corrupted by your
email client, turn off all encodings etc. in your client, send
a test patch to yourself, and do not submit this patch again until
you can successfully apply the patch you receive in those test emails.
Thanks.
^ permalink raw reply
* Re: pull request: wireless 2013-10-15
From: David Miller @ 2013-10-17 20:06 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131015175611.GA15706-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Tue, 15 Oct 2013 13:56:12 -0400
> Please pull this batch of fixes intended for the 3.12 stream!
>
> For the mac80211 bits, Johannes says:
>
> "Jouni fixes a remain-on-channel vs. scan bug, and Felix fixes client TX
> probing on VLANs."
>
> And also:
>
> "This time I have two fixes from Emmanuel for RF-kill issues, and fixed
> two issues reported by Evan Huus and Thomas Lindroth respectively."
>
> On top of those...
>
> Avinash Patil adds a couple of mwifiex fixes to properly inform cfg80211
> about some different types of disconnects, avoiding WARNINGs.
>
> Mark Cave-Ayland corrects a pointer arithmetic problem in rtlwifi,
> avoiding incorrect automatic gain calculations.
>
> Solomon Peachy sends a cw1200 fix for locking around calls to
> cw1200_irq_handler, addressing "lost interrupt" problems.
>
> Please let me know if there are problems!
Pulled, thanks a lot John.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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
* Re: [PATCH 1/2] tcp: must unclone packets before mangling them
From: David Miller @ 2013-10-17 20:08 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, ncardwell, ycheng
In-Reply-To: <1381863270.2045.62.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 15 Oct 2013 11:54:30 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> TCP stack should make sure it owns skbs before mangling them.
>
> We had various crashes using bnx2x, and it turned out gso_size
> was cleared right before bnx2x driver was populating TC descriptor
> of the _previous_ packet send. TCP stack can sometime retransmit
> packets that are still in Qdisc.
>
> Of course we could make bnx2x driver more robust (using
> ACCESS_ONCE(shinfo->gso_size) for example), but the bug is TCP stack.
>
> We have identified two points where skb_unclone() was needed.
>
> This patch adds a WARN_ON_ONCE() to warn us if we missed another
> fix of this kind.
>
> Kudos to Neal for finding the root cause of this bug. Its visible
> using small MSS.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH 2/2] tcp: remove the sk_can_gso() check from tcp_set_skb_tso_segs()
From: David Miller @ 2013-10-17 20:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, ncardwell, ycheng
In-Reply-To: <1381865094.2045.69.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 15 Oct 2013 12:24:54 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> sk_can_gso() should only be used as a hint in tcp_sendmsg() to build GSO
> packets in the first place. (As a performance hint)
>
> Once we have GSO packets in write queue, we can not decide they are no
> longer GSO only because flow now uses a route which doesn't handle
> TSO/GSO.
>
> Core networking stack handles the case very well for us, all we need
> is keeping track of packet counts in MSS terms, regardless of
> segmentation done later (in GSO or hardware)
>
> Right now, if tcp_fragment() splits a GSO packet in two parts,
> @left and @right, and route changed through a non GSO device,
> both @left and @right have pcount set to 1, which is wrong,
> and leads to incorrect packet_count tracking.
>
> This problem was added in commit d5ac99a648 ("[TCP]: skb pcount with MTU
> discovery")
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
Also applied and queued up for -stable, thanks everyone.
^ permalink raw reply
* Re: [PATCH] net bridge: remove unused field
From: David Miller @ 2013-10-17 20:10 UTC (permalink / raw)
To: jhs; +Cc: shemminger, netdev
In-Reply-To: <525C5DED.1080406@mojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Mon, 14 Oct 2013 17:11:09 -0400
> Trivial patch
I know it seems rediculous, but anything we export to userspace, as we
do in this UAPI header, could be potentially be used by some piece of
source out there.
I'd rather just leave it be, you can add a comment saying it's unused
if you wish, but I'd really rather not remove it and run the risk of
breaking someone's build out there.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] ipv4: shrink rt_cache_stat
From: David Miller @ 2013-10-17 20:11 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1381916944.2045.124.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 16 Oct 2013 02:49:04 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> Half of the rt_cache_stat fields are no longer used after IP
> route cache removal, lets shrink this per cpu area.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Looks good, applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
From: Kristian Evensen @ 2013-10-17 20:11 UTC (permalink / raw)
To: Julian Anastasov; +Cc: Network Development
In-Reply-To: <alpine.LFD.2.03.1310172245440.2016@ssi.bg>
Hi,
On Thu, Oct 17, 2013 at 9:52 PM, Julian Anastasov <ja@ssi.bg> wrote:
> IIRC, such notifications were not implemented
> to avoid storms to routing daemons. Not sure if this is
> still true.
Thanks for letting me know. I was wondering if this was the case, but
then I saw that the deletion of IPv6 routes are announced (when
running "ip link set dev X down") and assumed that message storms was
(is?) not considered a problem.
-Kristian
^ permalink raw reply
* Re: [PATCH] bridge: Correctly clamp MAX forward_delay when enabling STP
From: David Miller @ 2013-10-17 20:12 UTC (permalink / raw)
To: herbert; +Cc: vyasevic, netdev, shemminger
In-Reply-To: <20131016005657.GA5394@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 16 Oct 2013 08:56:57 +0800
> On Tue, Oct 15, 2013 at 02:57:45PM -0400, Vlad Yasevich wrote:
>> Commit be4f154d5ef0ca147ab6bcd38857a774133f5450
>> bridge: Clamp forward_delay when enabling STP
>> had a typo when attempting to clamp maximum forward delay.
>>
>> It is possible to set bridge_forward_delay to be higher then
>> permitted maximum when STP is off. When turning STP on, the
>> higher then allowed delay has to be clamed down to max value.
>>
>> CC: Herbert Xu <herbert@gondor.apana.org.au>
>> CC: Stephen Hemminger <shemminger@vyatta.com>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>
> Good catch!
>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
From: Kristian Evensen @ 2013-10-17 20:14 UTC (permalink / raw)
To: Joe Perches; +Cc: Network Development
In-Reply-To: <1382030610.22110.112.camel@joe-AO722>
Hi,
On Thu, Oct 17, 2013 at 7:23 PM, Joe Perches <joe@perches.com> wrote:
>
> Perhaps this memset should be moved into the
> list_for_each_entry_safe loop where cfg is used.
Thanks for your comment. I decided to put it there to avoid calling it
for each route that will be deleted. Since the struct is never
modified (only read in fib_rtmsg()) and we have the rtnl_lock, I
assumed it to be safe. Are there cases where this assumption is wrong?
-Kristian
^ permalink raw reply
* Re: pull request: wireless-next 2013-10-17
From: David Miller @ 2013-10-17 20:15 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev
In-Reply-To: <20131017182339.GC1812@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Thu, 17 Oct 2013 14:23:40 -0400
> This is a batch of updates intended for the 3.13 stream...
>
> The biggest item of interest in here is wcn36xx, the new mac80211
> driver for Qualcomm WCN3660/WCN3680 hardware.
>
> Regarding the mac80211 bits, Johannes says:
>
> "We have an assortment of cleanups and new features, of which the
> biggest one is probably the channel-switch support in IBSS. Nothing
> else really stands out much."
>
> On top of that, the ath9k and rt2x00 get a lot of update action from
> Felix Fietkau and Gabor Juhos, respectively. There are a handful of
> updates to other drivers here and there as well.
>
> Please let me know if there are problems!
Pulled, thanks John.
^ permalink raw reply
* Re: [PATCH net 2/9] bnx2x: Prevent an illegal pointer dereference during panic
From: David Miller @ 2013-10-17 20:15 UTC (permalink / raw)
To: eilong; +Cc: yuvalmin, netdev, ariele, dmitry
In-Reply-To: <1382035647.20308.3.camel@lb-tlvb-eilong.il.broadcom.com>
From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Thu, 17 Oct 2013 21:47:27 +0300
> I guess we should really re-spin and check for all 1's, or add some more
> code to make it really accurate, or at least fix the ">" to ">=". We
> will send a revised series with a fix.
Thanks Eilon.
^ permalink raw reply
* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
From: David Miller @ 2013-10-17 20:19 UTC (permalink / raw)
To: ja; +Cc: kristian.evensen, netdev
In-Reply-To: <alpine.LFD.2.03.1310172245440.2016@ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Thu, 17 Oct 2013 22:52:43 +0300 (EEST)
>
> Hello,
>
> On Thu, 17 Oct 2013, Kristian Evensen wrote:
>
>> From: Kristian Evensen <kristian.evensen@gmail.com>
>>
>> When a link is set as down using for example "ip link set dev X down", routes
>> are deleted, but RTM_DELROUTE messages are not sent to RTNLGRP_IPV4_ROUTE. This
>> patch makes trie_flush_list() send a RTM_DELROUTE for each route that is
>> removed, and makes rtnelink route deletion behavior consistent across commands.
>> The parameter lists for trie_flush_list() and trie_flush_leaf() are expanded to
>> include required information otherwise unavailable in trie_flush_list().
>>
>> One use case that is simplified by this patch is IPv4 WAN connection monitoring.
>> Instead of listening for and handling both RTM_*ROUTE and RTM_*LINK-messages, it
>> is sufficient to handle RTM_*ROUTE.
>
> IIRC, such notifications were not implemented
> to avoid storms to routing daemons. Not sure if this is
> still true.
I'm definitely not applying this patch.
Routing daemons maintain the routing table internally, and they
therefore can purge their internal data structures when the link
down event occurs.
This is how it has worked forever and for real BGP tables the
RTM_DELROUTE traffic would be enormous and at nearly DoS levels
for anyone listening for them.
^ permalink raw reply
* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
From: David Miller @ 2013-10-17 20:19 UTC (permalink / raw)
To: kristian.evensen; +Cc: ja, netdev
In-Reply-To: <CAKfDRXiM-8dnGqFMofdiaD=EMJNLxzx-yhVManRSiQHgziuVdQ@mail.gmail.com>
From: Kristian Evensen <kristian.evensen@gmail.com>
Date: Thu, 17 Oct 2013 22:11:39 +0200
> On Thu, Oct 17, 2013 at 9:52 PM, Julian Anastasov <ja@ssi.bg> wrote:
>> IIRC, such notifications were not implemented
>> to avoid storms to routing daemons. Not sure if this is
>> still true.
>
> Thanks for letting me know. I was wondering if this was the case, but
> then I saw that the deletion of IPv6 routes are announced (when
> running "ip link set dev X down") and assumed that message storms was
> (is?) not considered a problem.
I really wish IPV6 wouldn't be inconsistent and behave that way.
^ permalink raw reply
* Re: [PATCH v4 net 0/3] sctp: Use software checksum under certain
From: Vlad Yasevich @ 2013-10-17 20:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-sctp, fan.du
In-Reply-To: <20131017.152634.216002495730124314.davem@davemloft.net>
On 10/17/2013 03:26 PM, David Miller wrote:
> From: Vlad Yasevich <vyasevich@gmail.com>
> Date: Tue, 15 Oct 2013 22:01:28 -0400
>
>> There are some cards that support SCTP checksum offloading. When using
>> these cards with IPSec or forcing IP fragmentation of SCTP traffic,
>> the checksum is computed incorrectly due to the fact that xfrm and IP/IPv6
>> fragmentation code do not know that this is SCTP traffic and do not
>> know that checksum has to be computed differently.
>>
>> To fix this, we let SCTP detect these conditions and perform software
>> checksum calculation.
>
> Series applied, thanks Vlad.
>
Hi David
Could you please queue this to stable as well.
Thanks
-vlad
^ permalink raw reply
* [PATCH net-next] fib: Use const struct nl_info * in rtmsg_fib
From: Joe Perches @ 2013-10-17 20:34 UTC (permalink / raw)
To: Kristian Evensen; +Cc: Network Development
In-Reply-To: <CAKfDRXjctGkcqSt9cM=iWbckLt4=7n8434WAA8rr4_cEofEO=Q@mail.gmail.com>
The rtmsg_fib function doesn't modify this argument so mark
it const.
Signed-off-by: Joe Perches <joe@perches.com>
---
On Thu, 2013-10-17 at 22:14 +0200, Kristian Evensen wrote:
> Hi,
Hi Kristian.
> On Thu, Oct 17, 2013 at 7:23 PM, Joe Perches <joe@perches.com> wrote:
> >
> > Perhaps this memset should be moved into the
> > list_for_each_entry_safe loop where cfg is used.
>
> Thanks for your comment. I decided to put it there to avoid calling it
> for each route that will be deleted. Since the struct is never
> modified (only read in fib_rtmsg()) and we have the rtnl_lock, I
> assumed it to be safe. Are there cases where this assumption is wrong?
I didn't look too hard.
I just glanced at the prototype and saw it wasn't const.
If rtmsg_fib() doesn't modify cfg (and it doesn't seem to)
then the prototype and function should be marked const.
And you're right, it doesn't need to be inside the loop.
cheers, Joe
net/ipv4/fib_lookup.h | 2 +-
net/ipv4/fib_semantics.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index af0f14a..50cfb3e 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -32,7 +32,7 @@ extern int fib_dump_info(struct sk_buff *skb, u32 pid, u32 seq, int event,
int dst_len, u8 tos, struct fib_info *fi,
unsigned int);
extern void rtmsg_fib(int event, __be32 key, struct fib_alias *fa,
- int dst_len, u32 tb_id, struct nl_info *info,
+ int dst_len, u32 tb_id, const struct nl_info *info,
unsigned int nlm_flags);
extern struct fib_alias *fib_find_alias(struct list_head *fah,
u8 tos, u32 prio);
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index d5dbca5..e63f47a 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -380,7 +380,7 @@ static inline size_t fib_nlmsg_size(struct fib_info *fi)
}
void rtmsg_fib(int event, __be32 key, struct fib_alias *fa,
- int dst_len, u32 tb_id, struct nl_info *info,
+ int dst_len, u32 tb_id, const struct nl_info *info,
unsigned int nlm_flags)
{
struct sk_buff *skb;
^ permalink raw reply related
* [PATCH net] net: unix: inherit SOCK_PASS{CRED,SEC} flags from socket to fix race
From: Daniel Borkmann @ 2013-10-17 20:51 UTC (permalink / raw)
To: davem; +Cc: netdev, Eric Dumazet, Eric W. Biederman
In-Reply-To: <cover.1382042286.git.dborkman@redhat.com>
In the case of credentials passing in unix stream sockets (dgram
sockets seem not affected), we get a rather sparse race after
commit 16e5726 ("af_unix: dont send SCM_CREDENTIALS by default").
We have a stream server on receiver side that requests credential
passing from senders (e.g. nc -U). Since we need to set SO_PASSCRED
on each spawned/accepted socket on server side to 1 first (as it's
not inherited), it can happen that in the time between accept() and
setsockopt() we get interrupted, the sender is being scheduled and
continues with passing data to our receiver. At that time SO_PASSCRED
is neither set on sender nor receiver side, hence in cmsg's
SCM_CREDENTIALS we get eventually pid:0, uid:65534, gid:65534
(== overflow{u,g}id) instead of what we actually would like to see.
On the sender side, here nc -U, the tests in maybe_add_creds()
invoked through unix_stream_sendmsg() would fail, as at that exact
time, as mentioned, the sender has neither SO_PASSCRED on his side
nor sees it on the server side, and we have a valid 'other' socket
in place. Thus, sender believes it would just look like a normal
connection, not needing/requesting SO_PASSCRED at that time.
As reverting 16e5726 would not be an option due to the significant
performance regression reported when having creds always passed,
one way/trade-off to prevent that would be to set SO_PASSCRED on
the listener socket and allow inheriting these flags to the spawned
socket on server side in accept(). It seems also logical to do so
if we'd tell the listener socket to pass those flags onwards, and
would fix the race.
Before, strace:
recvmsg(4, {msg_name(0)=NULL, msg_iov(1)=[{"blub\n", 4096}],
msg_controllen=32, {cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS{pid=0, uid=65534, gid=65534}},
msg_flags=0}, 0) = 5
After, strace:
recvmsg(4, {msg_name(0)=NULL, msg_iov(1)=[{"blub\n", 4096}],
msg_controllen=32, {cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS{pid=11580, uid=1000, gid=1000}},
msg_flags=0}, 0) = 5
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
---
net/unix/af_unix.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 86de99a..c1f403b 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1246,6 +1246,15 @@ static int unix_socketpair(struct socket *socka, struct socket *sockb)
return 0;
}
+static void unix_sock_inherit_flags(const struct socket *old,
+ struct socket *new)
+{
+ if (test_bit(SOCK_PASSCRED, &old->flags))
+ set_bit(SOCK_PASSCRED, &new->flags);
+ if (test_bit(SOCK_PASSSEC, &old->flags))
+ set_bit(SOCK_PASSSEC, &new->flags);
+}
+
static int unix_accept(struct socket *sock, struct socket *newsock, int flags)
{
struct sock *sk = sock->sk;
@@ -1280,6 +1289,7 @@ static int unix_accept(struct socket *sock, struct socket *newsock, int flags)
/* attach accepted sock to socket */
unix_state_lock(tsk);
newsock->state = SS_CONNECTED;
+ unix_sock_inherit_flags(sock, newsock);
sock_graft(tsk, newsock);
unix_state_unlock(tsk);
return 0;
--
1.8.3.1
^ permalink raw reply related
* Oficiální vyhlá?ení OZNÁMENÍ
From: undp_chevronpetroleum1 @ 2013-10-17 20:16 UTC (permalink / raw)
Číslo ?ar?e : ( UK- 209-634 , E- 931-51 )
Oficiální vyhlá?ení OZNÁMENÍ
To je Vám oznámit, ?e jste byl vybrán pro peně?ní cenu
1,600,000.00 GB kilo mezinárodních programů uskutečňovaných na 15.října
2013 ve Spojeném království Chevron ropy / OSN o rozvoj
Program ( UNDP) Chcete-li zahájit zpracování va?í cenu jste kontaktovat
va?e nároky důstojník prostřednictvím na?ich akreditovaných agentů Prize Přenos jak je uvedeno
ní?e :
Mr.D. Matt
Organizace spojených národů Komplexní
PE9 2YP , Londýn
Spojené království
E- mail: derickmatt@googlemail.com
Kontaktujte jej , a poskytne mu na tajnou PIN kódu x5pukg2013 a
Va?e referenční číslo pro UNDP : 62082013EA-UK/21 . Také se Vám doporučuje
aby mu na základě uvedených informací, co nejdříve :
NÁROK PO?ADAVEK :
1.Name v plné vý?i : , 2.Address : , 3.Phone/Fax : ,
S pozdravem ,
Mrs.Tulise hnědá
Program Coordinator
POZNÁMKA: Chci, abyste věděli, ?e máme tři ?ťastné výherce v České republice se stalo, ?e jeden z nich
bude i rady, které byste se obrátit na vý?e uvedené tvrzení důstojníka co nejdříve .
^ permalink raw reply
* Re: [PATCH net-next v3 3/9] static_key: WARN on usage before jump_label_init was called
From: Steven Rostedt @ 2013-10-17 21:19 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, linux-kernel, Peter Zijlstra, Andi Kleen
In-Reply-To: <1381987923-1524-4-git-send-email-hannes@stressinduktion.org>
On Thu, 17 Oct 2013 07:31:57 +0200
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
> Based on a patch from Andi Kleen.
I'm fine with the patch, but the change log needs a lot more work.
Like, why is this needed? I know, but does anyone else?
-- Steve
>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> include/linux/jump_label.h | 10 ++++++++++
> include/linux/jump_label_ratelimit.h | 2 ++
> init/main.c | 7 +++++++
> kernel/jump_label.c | 5 +++++
> 4 files changed, 24 insertions(+)
>
> diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
> index a507907..e96be72 100644
> --- a/include/linux/jump_label.h
> +++ b/include/linux/jump_label.h
> @@ -48,6 +48,13 @@
>
> #include <linux/types.h>
> #include <linux/compiler.h>
> +#include <linux/bug.h>
> +
> +extern bool static_key_initialized;
> +
> +#define STATIC_KEY_CHECK_USE() WARN(!static_key_initialized, \
> + "%s used before call to jump_label_init", \
> + __func__)
>
> #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
>
> @@ -128,6 +135,7 @@ struct static_key {
>
> static __always_inline void jump_label_init(void)
> {
> + static_key_initialized = true;
> }
>
> static __always_inline bool static_key_false(struct static_key *key)
> @@ -146,11 +154,13 @@ static __always_inline bool static_key_true(struct static_key *key)
>
> static inline void static_key_slow_inc(struct static_key *key)
> {
> + STATIC_KEY_CHECK_USE();
> atomic_inc(&key->enabled);
> }
>
> static inline void static_key_slow_dec(struct static_key *key)
> {
> + STATIC_KEY_CHECK_USE();
> atomic_dec(&key->enabled);
> }
>
> diff --git a/include/linux/jump_label_ratelimit.h b/include/linux/jump_label_ratelimit.h
> index 1137883..089f70f 100644
> --- a/include/linux/jump_label_ratelimit.h
> +++ b/include/linux/jump_label_ratelimit.h
> @@ -23,12 +23,14 @@ struct static_key_deferred {
> };
> static inline void static_key_slow_dec_deferred(struct static_key_deferred *key)
> {
> + STATIC_KEY_CHECK_USE();
> static_key_slow_dec(&key->key);
> }
> static inline void
> jump_label_rate_limit(struct static_key_deferred *key,
> unsigned long rl)
> {
> + STATIC_KEY_CHECK_USE();
> }
> #endif /* HAVE_JUMP_LABEL */
> #endif /* _LINUX_JUMP_LABEL_RATELIMIT_H */
> diff --git a/init/main.c b/init/main.c
> index af310af..27bbec1a 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -136,6 +136,13 @@ static char *execute_command;
> static char *ramdisk_execute_command;
>
> /*
> + * Used to generate warnings if static_key manipulation functions are used
> + * before jump_label_init is called.
> + */
> +bool static_key_initialized __read_mostly = false;
> +EXPORT_SYMBOL_GPL(static_key_initialized);
> +
> +/*
> * If set, this is an indication to the drivers that reset the underlying
> * device before going ahead with the initialization otherwise driver might
> * rely on the BIOS and skip the reset operation.
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index 297a924..9019f15 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -58,6 +58,7 @@ static void jump_label_update(struct static_key *key, int enable);
>
> void static_key_slow_inc(struct static_key *key)
> {
> + STATIC_KEY_CHECK_USE();
> if (atomic_inc_not_zero(&key->enabled))
> return;
>
> @@ -103,12 +104,14 @@ static void jump_label_update_timeout(struct work_struct *work)
>
> void static_key_slow_dec(struct static_key *key)
> {
> + STATIC_KEY_CHECK_USE();
> __static_key_slow_dec(key, 0, NULL);
> }
> EXPORT_SYMBOL_GPL(static_key_slow_dec);
>
> void static_key_slow_dec_deferred(struct static_key_deferred *key)
> {
> + STATIC_KEY_CHECK_USE();
> __static_key_slow_dec(&key->key, key->timeout, &key->work);
> }
> EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
> @@ -116,6 +119,7 @@ EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
> void jump_label_rate_limit(struct static_key_deferred *key,
> unsigned long rl)
> {
> + STATIC_KEY_CHECK_USE();
> key->timeout = rl;
> INIT_DELAYED_WORK(&key->work, jump_label_update_timeout);
> }
> @@ -212,6 +216,7 @@ void __init jump_label_init(void)
> key->next = NULL;
> #endif
> }
> + static_key_initialized = true;
> jump_label_unlock();
> }
>
^ permalink raw reply
* Re: [PATCH] can: add Renesas R-Car CAN driver
From: Sergei Shtylyov @ 2013-10-17 21:54 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: netdev, wg, linux-can, linux-sh
In-Reply-To: <52487991.6000209@pengutronix.de>
Hello.
On 09/29/2013 11:03 PM, Marc Kleine-Budde wrote:
Sorry for the belated reply -- was on vacations.
>> Add support for the CAN controller found in Renesas R-Car SoCs.
> Is there a public available datasheet for the CAN core?
I don't think so.
> What architecture are the Renesas R-Car SoCs? They're ARM, aren't they?
ARM/SH, to be precise.
> What's R-Car's status on device tree conversion?
Ongoing, but at quite an early stage yet.
> More comments inline
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
[...]
>> Index: linux-can-next/drivers/net/can/rcar_can.c
>> ===================================================================
>> --- /dev/null
>> +++ linux-can-next/drivers/net/can/rcar_can.c
>> @@ -0,0 +1,898 @@
[...]
>> +#define RCAR_CAN_MIER1 0x42C /* CANi Mailbox Interrupt Enable Register 1 */
>> +#define RCAR_CAN_MKR(n) ((n) < 2 ? 0x430 + 4 * (n) : 0x400 + 4 * ((n) - 2))
>> + /* CANi Mask Register */
>> +#define RCAR_CAN_MKIVLR0 0x438 /* CANi Mask Invalid Register 0 */
>> +#define RCAR_CAN_MIER0 0x43C /* CANi Mailbox Interrupt Enable Register 0 */
>> +
>> +#define RCAR_CAN_MCTL(n) (0x800 + (n)) /* CANi Message Control Register */
>> +#define RCAR_CAN_CTLR 0x840 /* CANi Control Register */
>> +#define RCAR_CAN_STR 0x842 /* CANi Status Register */
>> +#define RCAR_CAN_BCR 0x844 /* CANi Bit Configuration Register */
>> +#define RCAR_CAN_CLKR 0x847 /* CANi Clock Select Register */
>> +#define RCAR_CAN_EIER 0x84C /* CANi Error Interrupt Enable Register */
>> +#define RCAR_CAN_EIFR 0x84D /* CANi Err Interrupt Factor Judge Register */
>> +#define RCAR_CAN_RECR 0x84E /* CANi Receive Error Count Register */
>> +#define RCAR_CAN_TECR 0x84F /* CANi Transmit Error Count Register */
>> +#define RCAR_CAN_ECSR 0x850 /* CANi Error Code Store Register */
>> +#define RCAR_CAN_MSSR 0x852 /* CANi Mailbox Search Status Register */
>> +#define RCAR_CAN_MSMR 0x853 /* CANi Mailbox Search Mode Register */
>> +#define RCAR_CAN_TCR 0x858 /* CANi Test Control Register */
>> +#define RCAR_CAN_IER 0x860 /* CANi Interrupt Enable Register */
>> +#define RCAR_CAN_ISR 0x861 /* CANi Interrupt Status Register */
> You might consider using an enum for the register offsets.
Doesn't seem a good idea to me, given the parametrized macros above
(corresponding to sequentially numbered registers).
>> +
>> +/* Offsets of RCAR_CAN Mailbox Registers */
>> +#define MBX_HDR_OFFSET 0x0
>> +#define MBX_DLC_OFFSET 0x5
>> +#define MBX_DATA_OFFSET 0x6
> can you please add the RCAR_ prefix to all defines.
I think they would look clumsy but if you insist on *all* defines, I'd do
it. I'm getting rid of the #define's above based on Wolfgang's feedback.
[...]
>> +static inline u32 rcar_can_mbx_readl(struct rcar_can_priv *priv,
>> + u32 mbxno, u8 offset)
>> +{
>> + return rcar_can_readl(priv, RCAR_CAN_MBX_SIZE * mbxno + offset);
> Can you define hide the RCAR_CAN_MBX_SIZE * mbxno into a macro?
> Something like:
> RCAR_CAN_MBX(mbxno)
Used a structure there based on Wolfgang's suggestion.
[...]
>> +static void rcar_can_error(struct net_device *ndev)
>> +{
[...]
>> + eifr = rcar_can_readb(priv, RCAR_CAN_EIFR);
>> + if (eifr & EIFR_EWIF) {
>> + netdev_dbg(priv->ndev, "Error warning interrupt\n");
>> + priv->can.state = CAN_STATE_ERROR_WARNING;
>> + priv->can.can_stats.error_warning++;
>> + cf->can_id |= CAN_ERR_CRTL;
>> + if (rcar_can_readb(priv, RCAR_CAN_TECR) > 96)
>> + cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
>> + if (rcar_can_readb(priv, RCAR_CAN_RECR) > 96)
>> + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
>> + /* Clear interrupt condition */
>> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EWIF);
> The cast is not needed.
Unfortunately, I get a compiler warning without it.
>> + }
>> + if (eifr & EIFR_EPIF) {
>> + netdev_dbg(priv->ndev, "Error passive interrupt\n");
>> + priv->can.state = CAN_STATE_ERROR_PASSIVE;
>> + priv->can.can_stats.error_passive++;
>> + cf->can_id |= CAN_ERR_CRTL;
>> + if (rcar_can_readb(priv, RCAR_CAN_TECR) > 127)
>> + cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
>> + if (rcar_can_readb(priv, RCAR_CAN_RECR) > 127)
>> + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
>> + /* Clear interrupt condition */
>> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_EPIF);
>> + }
>> + if (eifr & EIFR_BOEIF) {
>> + netdev_dbg(priv->ndev, "Bus-off entry interrupt\n");
>> + priv->can.state = CAN_STATE_BUS_OFF;
>> + cf->can_id |= CAN_ERR_BUSOFF;
>> + /* Clear interrupt condition */
>> + rcar_can_writeb(priv, RCAR_CAN_EIFR, (u8)~EIFR_BOEIF);
>> + /* Disable all interrupts in bus-off to avoid int hog */
>> + rcar_can_writeb(priv, RCAR_CAN_EIER, 0);
>> + rcar_can_writeb(priv, RCAR_CAN_IER, 0);
>> + can_bus_off(ndev);
> How does the rcan recover from bus off?
Good question... this needs additional work.
[...]
>> +static irqreturn_t rcar_can_interrupt(int irq, void *dev_id)
>> +{
>> + struct net_device *ndev = (struct net_device *)dev_id;
>> + struct rcar_can_priv *priv = netdev_priv(ndev);
>> + struct net_device_stats *stats = &ndev->stats;
>> + u8 isr;
>> +
>> + isr = rcar_can_readb(priv, RCAR_CAN_ISR);
>> + if (isr & ISR_ERSF)
>> + rcar_can_error(ndev);
>> +
>> + if (isr & ISR_TXMF) {
>> + u32 ie_mask = 0;
>> +
>> + /* Set Transmit Mailbox Search Mode */
>> + rcar_can_writeb(priv, RCAR_CAN_MSMR, MSMR_TXMB);
> Can you please outline how to mailbox search mode works? What happens if
> you activate tx mailbox search mode here and rx mailbox search mode below?
Work of the search is not well documented. The controller has 4 search
modes, only one can be enabled at the moment.
I just don't know what happens if we get TX interrupt while handling RX.
Either we painlessly switch to TX search, finish it and return to RX search,
and it continues where we are interrupted... or we lose some RX packets.
>> + while (1) {
> I presonally don't like while (1) loops in an interrupt handler, can you
> rearange the code, so that you have an explizid loop termination?
It was done in order to avoid assignment in the *while* expression.
Although checkpatch.pl didn't complain about it, it does complain about such
in the *if* expression, so I thought it consistent to avoid this in the
*while* too.
>> + u8 mctl, mbx;
>> +
>> + mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
>> + if (mbx & MSSR_SEST)
>> + break;
>> + mbx &= MSSR_MBNST;
>> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
>> + /* Bits SENTDATA and TRMREQ cannot be
>> + * set to 0 simultaneously
>> + */
>> + mctl &= ~MCTL_TRMREQ;
>> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
>> + mctl &= ~MCTL_SENTDATA;
>> + /* Clear interrupt */
>> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx), mctl);
> Can you combine both writes to RCAR_CAN_MCTL(mbx) or does the hardware
> require two separate writes?
Yes, it does. Haven't you seen the comment above the writes?
>> + ie_mask |= BIT(mbx - FIRST_TX_MB);
>> + stats->tx_bytes += can_get_echo_skb(ndev,
>> + mbx - FIRST_TX_MB);
> Can you guarantee that you call can_get_echo_skb in the same order the
> frames get send?
Another good question...
[...]
>> + /* Disable mailbox interrupt, mark mailbox as free */
>> + if (ie_mask) {
>> + u32 mier1;
>> +
>> + spin_lock(&priv->mier_lock);
>> + mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
>> + rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 & ~ie_mask);
>> + spin_unlock(&priv->mier_lock);
>> + if (unlikely(netif_queue_stopped(ndev)))
>> + netif_wake_queue(ndev);
> You can call netif_wake_queue() unconditionally, it does the check for
> queue stopped anyway.
Indeed.
>> + }
>> + }
>> + if (isr & ISR_RXM1F) {
>> + if (napi_schedule_prep(&priv->napi)) {
>> + /* Disable Rx interrupts */
>> + rcar_can_writeb(priv, RCAR_CAN_IER,
>> + rcar_can_readb(priv, RCAR_CAN_IER) &
>> + ~IER_RXM1IE);
>> + __napi_schedule(&priv->napi);
>> + }
>> + }
>> + return IRQ_HANDLED;
> Do not return IRQ_HANDLED unconditionally. You should return IRQ_HANDLED
> only if there really was a interrupt for your device.
Yes, people have already complained about this. I'll add a check of the
interrupt status.
>> +static int rcar_can_set_bittiming(struct net_device *dev)
>> +{
>> + struct rcar_can_priv *priv = netdev_priv(dev);
>> + struct can_bittiming *bt = &priv->can.bittiming;
>> + u32 bcr;
>> + u16 ctlr;
>> + u8 clkr;
>> +
>> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
>> + if (ctlr & CTLR_SLPM) {
>> + /* Write to BCR in CAN reset mode or CAN halt mode */
>> + return -EBUSY;
> The framework guarantees that set_bittiming is only called when the
> interface is down.
Exactly. With the current driver code we get rcar_can_set_bittiming()
called when device is not opened. It is in the sleep mode and we can not touch
CTLR register in the sleep mode. I call this function when opening device
explicitly.
[...]
>> +static void rcar_can_start(struct net_device *ndev)
>> +{
>> + struct rcar_can_priv *priv = netdev_priv(ndev);
>> + u16 ctlr, n;
>> +
>> + /* Set controller to known mode:
>> + * - normal mailbox mode (no FIFO);
> Does the controller support FIFO?
Yes.
[...]
>> +static netdev_tx_t rcar_can_start_xmit(struct sk_buff *skb,
>> + struct net_device *ndev)
>> +{
[...]
>> + spin_lock_irqsave(&priv->mier_lock, flags);
>> + mier1 = rcar_can_readl(priv, RCAR_CAN_MIER1);
>> + if (unlikely(mier1 == ~0U)) {
>> + spin_unlock_irqrestore(&priv->mier_lock, flags);
>> + netif_stop_queue(ndev);
>> + return NETDEV_TX_BUSY;
>> + }
>> + rcar_can_writel(priv, RCAR_CAN_MIER1, mier1 | (mier1 + 1));
>> + spin_unlock_irqrestore(&priv->mier_lock, flags);
>> + mbxno = ffz(mier1) + FIRST_TX_MB;
>
> This smells fishy, for several reasons:
> The driver should guarantee that the frames are send in the same order
> as this function is called. If you pick the first free mailbox I suspect
> the hardware send the first mailbox ready to send. I don't know the
> hardware, but several CAN controllers I've worked with, function in that
> way.
There are two TX priority modes: ID priority transmit mode and mailbox
number transmit mode. Currently, ID priority mode is selected -- I didn't
realize that "the driver should guarantee that the frames are send in the same
order as this function is called". Still driver can work in the mailbox number
priority mode.
ffz(mier1) selects first free mailbox with lowest number. It will have
higher priority.
> Then you should rethink your flow control. You should call
> netif_stop_queue if all your hardware buffers are full, then the above
> NETDEV_TX_BUSY should not happen.
You mean that I should check the state of mailboxes at end of this
function? OK.
[...]
>> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbxno));
> Do you have to read mctl here?
Hm, possibly not. Need to re-check...
[...]
>> +static void rcar_can_rx_pkt(struct rcar_can_priv *priv, int mbx)
>> +{
[...]
>> + data = rcar_can_mbx_readl(priv, mbx, MBX_HDR_OFFSET);
>> + if (data & RCAR_CAN_IDE)
>> + cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG;
>> + else
>> + cf->can_id = (data >> RCAR_CAN_SID_SHIFT) & CAN_SFF_MASK;
>> + if (data & RCAR_CAN_RTR)
>> + cf->can_id |= CAN_RTR_FLAG;
>> +
>> + dlc = rcar_can_mbx_readb(priv, mbx, MBX_DLC_OFFSET);
>> + cf->can_dlc = get_can_dlc(dlc);
> Please don't copy data on received RTR frames.
OK.
>> + for (dlc = 0; dlc < cf->can_dlc; dlc++)
>> + cf->data[dlc] = rcar_can_mbx_readb(priv, mbx,
>> + MBX_DATA_OFFSET + dlc);
[...]
>> +static int rcar_can_rx_poll(struct napi_struct *napi, int quota)
>> +{
>> + struct rcar_can_priv *priv = container_of(napi,
>> + struct rcar_can_priv, napi);
>> + u32 num_pkts = 0;
> please make it an int
OK, overlooked this.
>> +
>> + /* Find mailbox */
>> + while (1) {
> please put the quota check into the while()
OK.
>> + u8 mctl, mbx;
>> +
>> + mbx = rcar_can_readb(priv, RCAR_CAN_MSSR);
>> + if (mbx & MSSR_SEST || num_pkts >= quota)
>> + break;
>> + mbx &= MSSR_MBNST;
>> + mctl = rcar_can_readb(priv, RCAR_CAN_MCTL(mbx));
>> + /* Clear interrupt */
>> + rcar_can_writeb(priv, RCAR_CAN_MCTL(mbx),
>> + mctl & ~MCTL_NEWDATA);
>> + rcar_can_rx_pkt(priv, mbx);
> Which instruction reenables the mailbox?
Good question. It's not clear from documentation. For sure we can toggle
RECREQ bit. My current understanding is: mailbox is not ready to receive while
MCTL.NEWDATA is set. If so, need to clear interrupt after receive. May need to
check MSGLOST flag -- need to find out how to report overwritten messages.
>> + /* All packets processed */
>> + if (num_pkts < quota) {
>> + u8 ier;
>> +
>> + napi_complete(napi);
>> + ier = rcar_can_readb(priv, RCAR_CAN_IER);
>> + rcar_can_writeb(priv, RCAR_CAN_IER, ier | IER_RXM1IE);
> I the hardware doesn't modify the IER register, you can work on a shadow
> copy and only write, but never need to read from the hardware.
OK.
[...]
>> +static int rcar_can_probe(struct platform_device *pdev)
>> +{
[...]
>> + pdata = pdev->dev.platform_data;
> please use dev_get_platdata()
OK. I saw the patches converting the drivers to using this helper.
>> + clk_enable(priv->clk);
> You are missing the clock's prepare step.
We are not supporting common clock framework yet, so this step should be a
NOP AFAIU.
> You should call
> clk_prepare_enable(). Please move clock_prepare_enable() to the open()
> (and the disable_unprepare() to the close() function) so tha the core is
> not powered as long as the interface is down. You might have to enable
> the clock in the rcar_can_get_berr_counter() as it may be called if the
> interface is still down.
OK, will do.
>> +
>> + ndev->netdev_ops = &rcar_can_netdev_ops;
>> + ndev->irq = irq;
>> + ndev->flags |= IFF_ECHO;
>> + priv->ndev = ndev;
>> + priv->reg_base = addr;
>> + priv->clock_select = pdata->clock_select;
>> + priv->can.clock.freq = clk_get_rate(priv->clk);
>> + priv->can.bittiming_const = &rcar_can_bittiming_const;
>> + priv->can.do_set_bittiming = rcar_can_set_bittiming;
> No need to set this callback, as you're calling rcar_can_set_bittiming
> explizidly.
OK.
>> + priv->can.do_set_mode = rcar_can_do_set_mode;
>> + priv->can.do_get_berr_counter = rcar_can_get_berr_counter;
>> + priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES |
>> + CAN_CTRLMODE_ONE_SHOT;
> You don't handle 3_SAMPLES. Have you tested your driver in ONE_SHOT
I've tested in ONE_SHOT but the controller was forced into that mode.
Didn't find application that sets this mode.
> mode? How does the controller react if a frame cannot be send? Do you
> clear the skb that has previously been can_put_echo_skb()?
Another thing to look at -- maybe related to bus-off question.
>> +static int rcar_can_remove(struct platform_device *pdev)
>> +{
>> + struct net_device *ndev = platform_get_drvdata(pdev);
>> + struct rcar_can_priv *priv = netdev_priv(ndev);
>> + u16 ctlr;
>> +
>> + unregister_candev(ndev);
>> + netif_napi_del(&priv->napi);
>> + /* Go to sleep mode */
>> + ctlr = rcar_can_readw(priv, RCAR_CAN_CTLR);
>> + rcar_can_writew(priv, RCAR_CAN_CTLR, ctlr | CTLR_SLPM);
> You should put the controller in to sleep mode in close()
Seems OK to do this.
[...]
>> Index: linux-can-next/include/linux/can/platform/rcar_can.h
>> ===================================================================
>> --- /dev/null
>> +++ linux-can-next/include/linux/can/platform/rcar_can.h
>> @@ -0,0 +1,15 @@
>> +#ifndef _CAN_PLATFORM_RCAR_CAN_H_
>> +#define _CAN_PLATFORM_RCAR_CAN_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/* Clock Select Register settings */
>> +#define CLKR_CLKEXT 3 /* Externally input clock */
>> +#define CLKR_CLKP2 1 /* Peripheral clock (clkp2) */
>> +#define CLKR_CLKP1 0 /* Peripheral clock (clkp1) */
> Can this be handled by the clock framework
No.
> and or Device Tree?
Yes.
> Marc
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] can: add Renesas R-Car CAN driver
From: Sergei Shtylyov @ 2013-10-17 22:16 UTC (permalink / raw)
To: Wolfgang Grandegger, netdev, mkl, linux-can; +Cc: linux-sh, vksavl
In-Reply-To: <524BB883.2040400@grandegger.com>
Hello.
On 10/02/2013 10:09 AM, Wolfgang Grandegger wrote:
Sorry for the belated reply -- was on vacations.
> thanks for your contribution. The patch looks already quite good. Before
> I find time for a detailed review could you please check error handling
> and bus-off recovery by reporting the output of "$ candump -td -e
> any,0:0,#FFFFFFFF" while sending messages to the device ...
> 1. ... without cable connected
Terminal 1:
root@10.0.0.101:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.101:/opt/can-utils#
Terminal 2:
root@10.0.0.101:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
(000.000000) can0 200000AC [8] 00 08 00 19 00 00 00 00 ERRORFRAME
controller-problem{tx-error-warning}
protocol-violation{{}{acknowledge-slot}}
no-acknowledgement-on-tx
bus-error
(000.004496) can0 20000004 [8] 00 20 00 00 00 00 00 00 ERRORFRAME
controller-problem{tx-error-passive}
So we get and stay in error- passive state:
root@10.0.0.101:/opt/can-utils# ip -details link show can0
2: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN qlen
10 link/can
can state ERROR-PASSIVE (berr-counter tx 128 rx 0) restart-ms 0
bitrate 297619 sample-point 0.714
tq 480 prop-seg 2 phase-seg1 2 phase-seg2 2 sjw 1
rcar_can: tseg1 4..16 tseg2 2..8 sjw 1..4 brp 1..1024 brp-inc 1
clock 49999999
root@10.0.0.101:/opt/can-utils#
dmesg:
rcar_can rcar_can.0 can0: bitrate error 0.7%
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: ACK Error
rcar_can rcar_can.0 can0: Error passive interrupt
> 2. ... with short-circuited CAN high and low and doing some time later
> a manual recovery with "ip link set can0 type can restart"
Now we have auto recovery only. Manual recovery was tested with the first
driver version and worked.
Terminal 1:
root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils# ./cangen -n 1 -g 1 can0
root@10.0.0.104:/opt/can-utils#
Terminal 2:
root@10.0.0.104:/opt/can-utils# ./candump -td -e any,0:0,#FFFFFFFF
(000.000000) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021147) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off
(011.738522) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021163) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off
(001.666625) can0 2000008C [8] 00 00 08 00 00 00 00 00 ERRORFRAME
controller-problem{}
protocol-violation{{tx-dominant-bit-error}{}}
bus-error
(000.021157) can0 20000144 [8] 00 00 00 00 00 00 00 00 ERRORFRAME
controller-problem{}
bus-off
restarted-after-bus-off
dmesg:
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus error interrupt:
rcar_can rcar_can.0 can0: Bit Error (dominant)
rcar_can rcar_can.0 can0: Error warning interrupt
rcar_can rcar_can.0 can0: Error passive interrupt
rcar_can rcar_can.0 can0: Bus-off entry interrupt
rcar_can rcar_can.0 can0: bus-off
rcar_can rcar_can.0 can0: Bus-off recovery interrupt
> I also wonder if the messages are always sent in order. You could use
> the program "canfdtest" [1] from the can-utils for validation.
This program is PITA. With the driver workaroung it works:
FULL Duplex test:
on DUT:
root@10.0.0.104:/opt/can-utils# ./canfdtest -v can0
interface = can0, family = 29, type = 3, proto = 1
............................
root@am335x-evm:~# canconfig can0 bitrate 1000000
can0 bitrate: 1000000, sample-point: 0.750
root@am335x-evm:~# canconfig can0 start
[50164.566650] d_can d_can.1: can0: setting CAN BT = 0x2701
can0 state: ERROR-ACTIVE
root@am335x-evm:~# ./canfdtest -v -g can0
interface = can0, family = 29, type = 3, proto = 1
...............................................................................C
Test messages sent and received: 3826567
Exiting...
> Support for CAN_CTRLMODE_BERR_REPORTING would also be nice, especially
> if the bus errors reporting can easily be switched off (via interrupt mask).
Done.
> There is also the RCAN-TL1 (Renesas CAN Time Trigger Level 1) module
> e.g. available on the SH7203 Group of processors. Is this CAN controller
> compatible to some extend with the R-Car CAN or a completely different
> implementation?
No, not compatible.
> [1] https://gitorious.org/linux-can/can-utils/source/canfdtest.c
> Wolfgang.
WBR, Sergei
^ permalink raw reply
* [PATCH net] qlcnic: Validate Tx queue only for 82xx adapters.
From: Himanshu Madhani @ 2013-10-17 22:26 UTC (permalink / raw)
To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Himanshu Madhani
From: Himanshu Madhani <himanshu.madhani@qlogic.com>
o validate Tx queue only in case of adapters which supports
multi Tx queue.
This patch is to fix regression introduced in commit
aa4a1f7df7cbb98797c9f4edfde3c726e2b3841f
"qlcnic: Enable Tx queue changes using ethtool for 82xx Series adapter"
Signed-off-by: Himanshu Madhani <himanshu.madhani@qlogic.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c | 2 +-
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 8 +-------
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
index ebe4c86..ff83a9f 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
@@ -665,7 +665,7 @@ static int qlcnic_set_channels(struct net_device *dev,
return err;
}
- if (channel->tx_count) {
+ if (qlcnic_82xx_check(adapter) && channel->tx_count) {
err = qlcnic_validate_max_tx_rings(adapter, channel->tx_count);
if (err)
return err;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index f07f2b0..9e61eb8 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -3651,11 +3651,6 @@ int qlcnic_validate_max_tx_rings(struct qlcnic_adapter *adapter, u32 txq)
u8 max_hw = QLCNIC_MAX_TX_RINGS;
u32 max_allowed;
- if (!qlcnic_82xx_check(adapter)) {
- netdev_err(netdev, "No Multi TX-Q support\n");
- return -EINVAL;
- }
-
if (!qlcnic_use_msi_x && !qlcnic_use_msi) {
netdev_err(netdev, "No Multi TX-Q support in INT-x mode\n");
return -EINVAL;
@@ -3695,8 +3690,7 @@ int qlcnic_validate_max_rss(struct qlcnic_adapter *adapter,
u8 max_hw = adapter->ahw->max_rx_ques;
u32 max_allowed;
- if (qlcnic_82xx_check(adapter) && !qlcnic_use_msi_x &&
- !qlcnic_use_msi) {
+ if (!qlcnic_use_msi_x && !qlcnic_use_msi) {
netdev_err(netdev, "No RSS support in INT-x mode\n");
return -EINVAL;
}
--
1.8.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox