* Re: [PATCH net-next] be2net: support TX batching using skb->xmit_more flag
From: David Miller @ 2015-01-05 21:33 UTC (permalink / raw)
To: sathya.perla; +Cc: netdev
In-Reply-To: <1420454914-9516-1-git-send-email-sathya.perla@emulex.com>
From: Sathya Perla <sathya.perla@emulex.com>
Date: Mon, 5 Jan 2015 05:48:34 -0500
> This patch uses skb->xmit_more flag to batch TX requests.
> TX is flushed either when xmit_more is false or there is
> no more space in the TXQ.
>
> Skyhawk-R and BEx chips require an even number of wrbs to be posted.
> So, when a batch of TX requests is accumulated, the last header wrb
> may need to be fixed with an extra dummy wrb.
>
> This patch refactors be_xmit() routine as a sequence of be_xmit_enqueue()
> and be_xmit_flush() calls. The Tx completion code is also
> updated to be able to unmap/free a batch of skbs rather than a single
> skb.
>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
Applied.
^ permalink raw reply
* Re: [PATCH] at86rf230: Constify struct regmap_config
From: David Miller @ 2015-01-05 21:33 UTC (permalink / raw)
To: k.kozlowski; +Cc: alex.aring, linux-wpan, netdev, linux-kernel
In-Reply-To: <1420448551-9443-1-git-send-email-k.kozlowski@samsung.com>
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Date: Mon, 05 Jan 2015 10:02:31 +0100
> The regmap_config struct may be const because it is not modified by the
> driver and regmap_init() accepts pointer to const.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2] rhashtable: involve rhashtable_lookup_insert routine
From: David Miller @ 2015-01-05 21:30 UTC (permalink / raw)
To: tgraf; +Cc: ying.xue, netdev
In-Reply-To: <20150105130514.GA15499@casper.infradead.org>
From: Thomas Graf <tgraf@suug.ch>
Date: Mon, 5 Jan 2015 13:05:14 +0000
> On 01/05/15 at 07:33pm, Ying Xue wrote:
>> Involve a new function called rhashtable_lookup_insert() which makes
>> lookup and insertion atomic under bucket lock protection, helping us
>> avoid to introduce an extra lock when we search and insert an object
>> into hash table.
>>
>> Signed-off-by: Ying Xue <ying.xue@windriver.com>
>> Signed-off-by: Thomas Graf <tgraf@suug.ch>
>
> Thanks for putting this around so quickly and thanks for testing.
> I think this looks good. You might be able to factor out some code
> from rhashtable_insert() to avoid duplication so we reduce the risk
> of fixing a bug for one function but not the other.
Do you want Ying to do this factoring out now in a v3 of this patch
or in a subsequent patch?
I assume the former since you didn't give your ACK.
^ permalink raw reply
* Re: WIP alternative - was Re: [PATCH v3 14/20] selftests/size: add install target to enable test install
From: Shuah Khan @ 2015-01-05 21:28 UTC (permalink / raw)
To: Tim Bird, mmarek@suse.cz, gregkh@linuxfoundation.org,
akpm@linux-foundation.org, rostedt@goodmis.org, mingo@redhat.com,
davem@davemloft.net, keescook@chromium.org,
tranmanphong@gmail.com, mpe@ellerman.id.au, cov@codeaurora.org,
dh.herrmann@gmail.com, hughd@google.com, bobby.prani@gmail.com,
serge.hallyn@ubuntu.com, ebiederm@xmission.com,
josh@joshtriplett.org, koct9i@gmail.com
Cc: linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-api@vger.kernel.org, netdev@vger.kernel.org, Shuah Khan
In-Reply-To: <54A4B174.2010501@sonymobile.com>
On 12/31/2014 07:31 PM, Tim Bird wrote:
> On 12/24/2014 08:27 AM, Shuah Khan wrote:
>> Add a new make target to enable installing test. This target
>> installs test in the kselftest install location and add to the
>> kselftest script to run the test. Install target can be run
>> only from top level kernel source directory.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>> tools/testing/selftests/size/Makefile | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
>> index 04dc25e..bb7113b 100644
>> --- a/tools/testing/selftests/size/Makefile
>> +++ b/tools/testing/selftests/size/Makefile
>> @@ -1,12 +1,22 @@
>> CC = $(CROSS_COMPILE)gcc
>>
>> +TEST_STR = ./get_size || echo get_size selftests: [FAIL]
>> +
>> all: get_size
>>
>> get_size: get_size.c
>> $(CC) -static -ffreestanding -nostartfiles -s $< -o $@
>>
>> +install:
>> +ifdef INSTALL_KSFT_PATH
>> + install ./get_size $(INSTALL_KSFT_PATH)
>> + @echo "$(TEST_STR)" >> $(KSELFTEST)
>> +else
>> + @echo Run make kselftest_install in top level source directory
>> +endif
>> +
>> run_tests: all
>> - ./get_size
>> + @$(TEST_STR)
>>
>> clean:
>> $(RM) get_size
>>
>
> The install phase is desperately needed for usage of kselftest in
> cross-target situations (applicable to almost all embedded). So this
> is great stuff.
Thanks.
>
> I worked a bit on isolating the install stuff to a makefile include file.
> This allows simplifying some of the sub-level Makefiles a bit, and allowing
> control of some of the install and run logic in less places.
>
> This is still a work in progress, but before I got too far along, I wanted
> to post it for people to provide feedback. A couple of problems cropped
> up that are worth discussing, IMHO.
>
> 1) I think it should be a requirement that each test has a single
> "main" program to execute to run the tests. If multiple tests are supported
> or more flexibility is desired for additional arguments, or that sort of
> thing, then that's fine, but the automated script builder should be able
> to run just a single program or script to have things work. This also
> makes things more consistent. In the case of the firmware test, I created
> a single fw_both.sh script to do this, instead of having two separate
> blocks in the kselftest.sh script.
It is a good goal for individual tests to use a main program to run
tests, even though, I would not make it a requirement. I would like to
leave that decision up to the individual test writer.
>
> 2) I've added a CROSS_INSTALL variable, which can call an arbitrary program
> to place files on the target system (rather than just calling 'install').
> In my case, I'd use my own 'ttc cp' command, which I can extend as necessary
> to put things on a remote machine. This works for a single directory,
> but things get dicier with sub-directory trees full of files (like
> the ftrace test uses.)
>
> If additional items need to be installed to the target, then maybe a setup
> program should be used, rather than just copying files.
>
> 3) Some of the scripts were using /bin/bash to execute them, rather
> than rely on the interpreter line in the script itself (and having
> the script have executable privileges). Is there a reason for this?
> I modified a few scripts to be executable, and got rid of the
> explicit execution with /bin/bash.
Probably no reason other than the choice made by the test writer.
It could be cleaned up and made consistent, however, I would see
this as a separate enhancement type work that could be done on its
own and not include it in the install work.
>
> The following is just a start... Let me know if this direction looks
> OK, and I'll finish this up. The main item to look at is
> kselftest.include file. Note that these patches are based on Shuah's
> series - but if you want to use these ideas I can rebase them onto
> mainline, and break them out per test sub-level like Shuah did.
One of the reasons I picked install target approach is to enable the
feature by extending the existing run_tests support. This way we will
have the feature available quickly. Once that is supported, we can work
on evolving to a generic approach to use the include file approach, as
the changes you are proposing are based on the the series I sent out,
and makes improvements to it.
kselftest.include file approach could work for several tests and tests
that can't use the generic could add their own install support.
I propose evolving to a generic kselftest.include as the second step in
evolving the install feature. Can I count on you do the work and update
the tests to use kselftest.include, CROSS_INSTALL variable support?
thanks,
-- Shuah
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* Re: [PATCH/RFC rocker-net-next 1/6] net: flow: Cancel innermost nested attribute first
From: David Miller @ 2015-01-05 21:17 UTC (permalink / raw)
To: simon.horman; +Cc: john.fastabend, netdev
In-Reply-To: <1420440610-20621-2-git-send-email-simon.horman@netronome.com>
From: Simon Horman <simon.horman@netronome.com>
Date: Mon, 5 Jan 2015 15:50:05 +0900
> Cancel innermost nested attribute first on error when putting flow actions.
>
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
>
> ---
>
> Its unclear to me if this makes any difference.
> But it seems more logical to me.
Hmmm. Be careful here. nla_nest_cancel() is just rolling back
the length of the SKB to right before the netlink attribute being
given as the cancellation point.
So you really have to cancel attributes in exactly the reverse order
in which they were added. Otherwise we'll make a trim call with a
negative adjustment that actually expands the SKB past an already
cancelled attribute.
^ permalink raw reply
* Re: [PATCH net-next 4/4] ip: Add support for IP_CHECKSUM cmsg
From: David Miller @ 2015-01-05 21:12 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <1420488035-24866-5-git-send-email-therbert@google.com>
From: Tom Herbert <therbert@google.com>
Date: Mon, 5 Jan 2015 12:00:35 -0800
> diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
> index 4091fab..2823fc0 100644
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@ -203,6 +203,7 @@ struct inet_sock {
> #define IP_CMSG_RETOPTS (1 << 4)
> #define IP_CMSG_PASSSEC (1 << 5)
> #define IP_CMSG_ORIGDSTADDR (1 << 6)
> +#define IP_CMSG_CHECKSUM (1 << 7)
Likewise, please include linux/bitops.h and use "BIT()" for this.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 0/4] ip: Support checksum returned in csmg
From: David Miller @ 2015-01-05 21:11 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <1420488035-24866-1-git-send-email-therbert@google.com>
From: Tom Herbert <therbert@google.com>
Date: Mon, 5 Jan 2015 12:00:31 -0800
> This patch set allows the packet checksum for a datagram socket
> to be returned in csum data in recvmsg. This allows userspace
> to implement its own checksum over the data, for instance if an
> IP tunnel was be implemented in user space, the inner checksum
> could be validated.
>
> Changes in this patch set:
> - Move checksum conversion to inet_sock from udp_sock. This
> generalizes checksum conversion for use with other protocols.
> - Move IP cmsg constants to a header file and make processing
> of the flags more efficient in ip_cmsg_recv
> - Return checksum value in cmsg. This is specifically the unfolded
> 32 bit checksum of the full packet starting from the first byte
> returned in recvmsg
>
> Tested: Wrote a little server to get checksums in cmsg for UDP and
> verfied correct checksum is returned.
This series looks fine, I just want two minor adjustments. I'll
reply to the relevant patches.
^ permalink raw reply
* [PATCH net-next 4/4] ip: Add support for IP_CHECKSUM cmsg
From: Tom Herbert @ 2015-01-05 20:00 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1420488035-24866-1-git-send-email-therbert@google.com>
New cmsg type is IP_CHECKSUM under SOL_IP. Enabled by standard
setsockopt.
The value returned is the unfolded 32 bit checksum of the packet
being received starting from the first byte returned in recvmsg
through the end of the packet (truncation is disregarded).
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/net/inet_sock.h | 1 +
include/uapi/linux/in.h | 1 +
net/ipv4/ip_sockglue.c | 41 ++++++++++++++++++++++++++++++++++++++++-
net/ipv4/udp.c | 2 +-
4 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 4091fab..2823fc0 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -203,6 +203,7 @@ struct inet_sock {
#define IP_CMSG_RETOPTS (1 << 4)
#define IP_CMSG_PASSSEC (1 << 5)
#define IP_CMSG_ORIGDSTADDR (1 << 6)
+#define IP_CMSG_CHECKSUM (1 << 7)
static inline struct inet_sock *inet_sk(const struct sock *sk)
{
diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
index c33a65e..589ced0 100644
--- a/include/uapi/linux/in.h
+++ b/include/uapi/linux/in.h
@@ -109,6 +109,7 @@ struct in_addr {
#define IP_MINTTL 21
#define IP_NODEFRAG 22
+#define IP_CHECKSUM 23
/* IP_MTU_DISCOVER values */
#define IP_PMTUDISC_DONT 0 /* Never send DF frames */
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 513d506..a317797 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -37,6 +37,7 @@
#include <net/route.h>
#include <net/xfrm.h>
#include <net/compat.h>
+#include <net/checksum.h>
#if IS_ENABLED(CONFIG_IPV6)
#include <net/transp_v6.h>
#endif
@@ -96,6 +97,20 @@ static void ip_cmsg_recv_retopts(struct msghdr *msg, struct sk_buff *skb)
put_cmsg(msg, SOL_IP, IP_RETOPTS, opt->optlen, opt->__data);
}
+static void ip_cmsg_recv_checksum(struct msghdr *msg, struct sk_buff *skb,
+ int offset)
+{
+ __wsum csum = skb->csum;
+
+ if (skb->ip_summed != CHECKSUM_COMPLETE)
+ return;
+
+ if (offset != 0)
+ csum = csum_sub(csum, csum_partial(skb->data, offset, 0));
+
+ put_cmsg(msg, SOL_IP, IP_CHECKSUM, sizeof(__wsum), &csum);
+}
+
static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
{
char *secdata;
@@ -191,9 +206,16 @@ void ip_cmsg_recv_offset(struct msghdr *msg, struct sk_buff *skb,
return;
}
- if (flags & IP_CMSG_ORIGDSTADDR)
+ if (flags & IP_CMSG_ORIGDSTADDR) {
ip_cmsg_recv_dstaddr(msg, skb);
+ flags &= ~IP_CMSG_ORIGDSTADDR;
+ if (!flags)
+ return;
+ }
+
+ if (flags & IP_CMSG_CHECKSUM)
+ ip_cmsg_recv_checksum(msg, skb, offset);
}
EXPORT_SYMBOL(ip_cmsg_recv_offset);
@@ -533,6 +555,7 @@ static int do_ip_setsockopt(struct sock *sk, int level,
case IP_MULTICAST_ALL:
case IP_MULTICAST_LOOP:
case IP_RECVORIGDSTADDR:
+ case IP_CHECKSUM:
if (optlen >= sizeof(int)) {
if (get_user(val, (int __user *) optval))
return -EFAULT;
@@ -630,6 +653,19 @@ static int do_ip_setsockopt(struct sock *sk, int level,
else
inet->cmsg_flags &= ~IP_CMSG_ORIGDSTADDR;
break;
+ case IP_CHECKSUM:
+ if (val) {
+ if (!(inet->cmsg_flags & IP_CMSG_CHECKSUM)) {
+ inet_inc_convert_csum(sk);
+ inet->cmsg_flags |= IP_CMSG_CHECKSUM;
+ }
+ } else {
+ if (inet->cmsg_flags & IP_CMSG_CHECKSUM) {
+ inet_dec_convert_csum(sk);
+ inet->cmsg_flags &= ~IP_CMSG_CHECKSUM;
+ }
+ }
+ break;
case IP_TOS: /* This sets both TOS and Precedence */
if (sk->sk_type == SOCK_STREAM) {
val &= ~INET_ECN_MASK;
@@ -1233,6 +1269,9 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
case IP_RECVORIGDSTADDR:
val = (inet->cmsg_flags & IP_CMSG_ORIGDSTADDR) != 0;
break;
+ case IP_CHECKSUM:
+ val = (inet->cmsg_flags & IP_CMSG_CHECKSUM) != 0;
+ break;
case IP_TOS:
val = inet->tos;
break;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 53358d8..97ef1f8b 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1329,7 +1329,7 @@ try_again:
*addr_len = sizeof(*sin);
}
if (inet->cmsg_flags)
- ip_cmsg_recv(msg, skb);
+ ip_cmsg_recv_offset(msg, skb, sizeof(struct udphdr));
err = copied;
if (flags & MSG_TRUNC)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 3/4] ip: Add offset parameter to ip_cmsg_recv
From: Tom Herbert @ 2015-01-05 20:00 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1420488035-24866-1-git-send-email-therbert@google.com>
Add ip_cmsg_recv_offset function which takes an offset argument
that indicates the starting offset in skb where data is being received
from. This will be useful in the case of UDP and provided checksum
to user space.
ip_cmsg_recv is an inline call to ip_cmsg_recv_offset with offset of
zero.
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/net/ip.h | 7 ++++++-
net/ipv4/ip_sockglue.c | 5 +++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 0bb6207..0e5a0ba 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -537,7 +537,7 @@ int ip_options_rcv_srr(struct sk_buff *skb);
*/
void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb);
-void ip_cmsg_recv(struct msghdr *msg, struct sk_buff *skb);
+void ip_cmsg_recv_offset(struct msghdr *msg, struct sk_buff *skb, int offset);
int ip_cmsg_send(struct net *net, struct msghdr *msg,
struct ipcm_cookie *ipc, bool allow_ipv6);
int ip_setsockopt(struct sock *sk, int level, int optname, char __user *optval,
@@ -557,6 +557,11 @@ void ip_icmp_error(struct sock *sk, struct sk_buff *skb, int err, __be16 port,
void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 dport,
u32 info);
+static inline void ip_cmsg_recv(struct msghdr *msg, struct sk_buff *skb)
+{
+ ip_cmsg_recv_offset(msg, skb, 0);
+}
+
bool icmp_global_allow(void);
extern int sysctl_icmp_msgs_per_sec;
extern int sysctl_icmp_msgs_burst;
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 80f7856..513d506 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -136,7 +136,8 @@ static void ip_cmsg_recv_dstaddr(struct msghdr *msg, struct sk_buff *skb)
put_cmsg(msg, SOL_IP, IP_ORIGDSTADDR, sizeof(sin), &sin);
}
-void ip_cmsg_recv(struct msghdr *msg, struct sk_buff *skb)
+void ip_cmsg_recv_offset(struct msghdr *msg, struct sk_buff *skb,
+ int offset)
{
struct inet_sock *inet = inet_sk(skb->sk);
unsigned int flags = inet->cmsg_flags;
@@ -194,7 +195,7 @@ void ip_cmsg_recv(struct msghdr *msg, struct sk_buff *skb)
ip_cmsg_recv_dstaddr(msg, skb);
}
-EXPORT_SYMBOL(ip_cmsg_recv);
+EXPORT_SYMBOL(ip_cmsg_recv_offset);
int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc,
bool allow_ipv6)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 1/4] ip: Move checksum convert defines to inet
From: Tom Herbert @ 2015-01-05 20:00 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1420488035-24866-1-git-send-email-therbert@google.com>
Move convert_csum from udp_sock to inet_sock. This allows the
possibility that we can use convert checksum for different types
of sockets and also allows convert checksum to be enabled from
inet layer (what we'll want to do when enabling IP_CHECKSUM cmsg).
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/linux/udp.h | 16 +---------------
include/net/inet_sock.h | 17 +++++++++++++++++
net/ipv4/fou.c | 2 +-
net/ipv4/udp.c | 2 +-
net/ipv4/udp_tunnel.c | 2 +-
net/ipv6/udp.c | 2 +-
6 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/include/linux/udp.h b/include/linux/udp.h
index ee32775..247cfdc 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -49,11 +49,7 @@ struct udp_sock {
unsigned int corkflag; /* Cork is required */
__u8 encap_type; /* Is this an Encapsulation socket? */
unsigned char no_check6_tx:1,/* Send zero UDP6 checksums on TX? */
- no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
- convert_csum:1;/* On receive, convert checksum
- * unnecessary to checksum complete
- * if possible.
- */
+ no_check6_rx:1;/* Allow zero UDP6 checksums on RX? */
/*
* Following member retains the information to create a UDP header
* when the socket is uncorked.
@@ -102,16 +98,6 @@ static inline bool udp_get_no_check6_rx(struct sock *sk)
return udp_sk(sk)->no_check6_rx;
}
-static inline void udp_set_convert_csum(struct sock *sk, bool val)
-{
- udp_sk(sk)->convert_csum = val;
-}
-
-static inline bool udp_get_convert_csum(struct sock *sk)
-{
- return udp_sk(sk)->convert_csum;
-}
-
#define udp_portaddr_for_each_entry(__sk, node, list) \
hlist_nulls_for_each_entry(__sk, node, list, __sk_common.skc_portaddr_node)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index a829b77..360b110 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -184,6 +184,7 @@ struct inet_sock {
mc_all:1,
nodefrag:1;
__u8 rcv_tos;
+ __u8 convert_csum;
int uc_index;
int mc_index;
__be32 mc_addr;
@@ -250,4 +251,20 @@ static inline __u8 inet_sk_flowi_flags(const struct sock *sk)
return flags;
}
+static inline void inet_inc_convert_csum(struct sock *sk)
+{
+ inet_sk(sk)->convert_csum++;
+}
+
+static inline void inet_dec_convert_csum(struct sock *sk)
+{
+ if (inet_sk(sk)->convert_csum > 0)
+ inet_sk(sk)->convert_csum--;
+}
+
+static inline bool inet_get_convert_csum(struct sock *sk)
+{
+ return !!inet_sk(sk)->convert_csum;
+}
+
#endif /* _INET_SOCK_H */
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index b986298..2197c36 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -490,7 +490,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
sk->sk_user_data = fou;
fou->sock = sock;
- udp_set_convert_csum(sk, true);
+ inet_inc_convert_csum(sk);
sk->sk_allocation = GFP_ATOMIC;
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 13b4dcf..53358d8 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1806,7 +1806,7 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
if (sk != NULL) {
int ret;
- if (udp_sk(sk)->convert_csum && uh->check && !IS_UDPLITE(sk))
+ if (inet_get_convert_csum(sk) && uh->check && !IS_UDPLITE(sk))
skb_checksum_try_convert(skb, IPPROTO_UDP, uh->check,
inet_compute_pseudo);
diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c
index 1671263..9996e63 100644
--- a/net/ipv4/udp_tunnel.c
+++ b/net/ipv4/udp_tunnel.c
@@ -63,7 +63,7 @@ void setup_udp_tunnel_sock(struct net *net, struct socket *sock,
inet_sk(sk)->mc_loop = 0;
/* Enable CHECKSUM_UNNECESSARY to CHECKSUM_COMPLETE conversion */
- udp_set_convert_csum(sk, true);
+ inet_inc_convert_csum(sk);
rcu_assign_sk_user_data(sk, cfg->sk_user_data);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 189dc4a..e41f017 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -909,7 +909,7 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
goto csum_error;
}
- if (udp_sk(sk)->convert_csum && uh->check && !IS_UDPLITE(sk))
+ if (inet_get_convert_csum(sk) && uh->check && !IS_UDPLITE(sk))
skb_checksum_try_convert(skb, IPPROTO_UDP, uh->check,
ip6_compute_pseudo);
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 0/4] ip: Support checksum returned in csmg
From: Tom Herbert @ 2015-01-05 20:00 UTC (permalink / raw)
To: davem, netdev
This patch set allows the packet checksum for a datagram socket
to be returned in csum data in recvmsg. This allows userspace
to implement its own checksum over the data, for instance if an
IP tunnel was be implemented in user space, the inner checksum
could be validated.
Changes in this patch set:
- Move checksum conversion to inet_sock from udp_sock. This
generalizes checksum conversion for use with other protocols.
- Move IP cmsg constants to a header file and make processing
of the flags more efficient in ip_cmsg_recv
- Return checksum value in cmsg. This is specifically the unfolded
32 bit checksum of the full packet starting from the first byte
returned in recvmsg
Tested: Wrote a little server to get checksums in cmsg for UDP and
verfied correct checksum is returned.
Tom Herbert (4):
ip: Move checksum convert defines to inet
ip: IP cmsg cleanup
ip: Add offset parameter to ip_cmsg_recv
ip: Add support for IP_CHECKSUM cmsg
include/linux/udp.h | 16 +------
include/net/inet_sock.h | 27 ++++++++++++
include/net/ip.h | 7 +++-
include/uapi/linux/in.h | 1 +
net/ipv4/fou.c | 2 +-
net/ipv4/ip_sockglue.c | 108 +++++++++++++++++++++++++++++++++++-------------
net/ipv4/udp.c | 4 +-
net/ipv4/udp_tunnel.c | 2 +-
net/ipv6/udp.c | 2 +-
9 files changed, 119 insertions(+), 50 deletions(-)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply
* next-timestamp build failure with 3.19-rc2
From: Vinson Lee @ 2015-01-05 19:55 UTC (permalink / raw)
To: Jonathan Corbet, David S. Miller, Willem de Bruijn
Cc: linux-doc, LKML, Netdev
Hi.
I'm hitting the following build error with 3.19-rc2 on CentOS 5 with
glibc-headers-2.5-123.
HOSTCC Documentation/networking/timestamping/txtimestamp
Documentation/networking/timestamping/txtimestamp.c:64:8: error:
redefinition of ‘struct in6_pktinfo’
struct in6_pktinfo {
^
In file included from /usr/include/arpa/inet.h:23:0,
from Documentation/networking/timestamping/txtimestamp.c:33:
/usr/include/netinet/in.h:456:8: note: originally defined here
struct in6_pktinfo
^
The build error was introduced with
cbd3aad5ce66f5a266a185aa37e0eb9be9ba4154 "net-timestamp: expand
documentation and test".
commit cbd3aad5ce66f5a266a185aa37e0eb9be9ba4154
Author: Willem de Bruijn <willemb@google.com>
Date: Sun Nov 30 22:22:35 2014 -0500
net-timestamp: expand documentation and test
Documentation:
expand explanation of timestamp counter
Test:
new: flag -I requests and prints PKTINFO
new: flag -x prints payload (possibly truncated)
fix: remove pretty print that breaks common flag '-l 1'
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Cheers,
Vinson
^ permalink raw reply
* Re: NULL pointer dereference at skb_queue_tail()
From: Cong Wang @ 2015-01-05 19:03 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: netdev
In-Reply-To: <201501052150.CIG69242.FFVOLQOJFHtMSO@I-love.SAKURA.ne.jp>
On Mon, Jan 5, 2015 at 4:50 AM, Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> Tetsuo Handa wrote:
>> I can reproduce below oops when testing Linux 3.18 with memory allocation
>> failure injection module at https://lkml.org/lkml/2014/12/25/64 .
>
> I can reliably reproduce this oops with current linux.git using memory
> allocation failure injection module. There is a possibility of memory
> corruption since this oops always occurs immediately after memory
> allocation failure within GPU/DRM code. I want to check whether
> fields of structures have expected values or not.
Looks like the skb->prev and/or skb->next in the skb queue is corrupted,
but I don't see why. We do play some magic on these pointers recently,
but it should not be related with unix socket at all.
Is it possible for you to check if this is a regression of recent kernel?
We only have few changes in unix socket recently, and I don't see they
could cause this bug.
>
>> void skb_queue_tail(struct sk_buff_head *list, struct sk_buff *newsk)
>> {
>> unsigned long flags;
>>
>
> Could you tell me what are expected values (i.e. what BUG_ON() test
> should I try) at this location?
>
Since skb queue has its own code to do list operations, we can't
use the existing list debugging to debug this list corruption. :(
^ permalink raw reply
* Re: [net-next PATCH v1 01/11] net: flow_table: create interface for hw match/action tables
From: John Fastabend @ 2015-01-05 18:59 UTC (permalink / raw)
To: Thomas Graf; +Cc: sfeldma, jiri, jhs, simon.horman, netdev, davem, andy
In-Reply-To: <20150104111238.GD15305@casper.infradead.org>
On 01/04/2015 03:12 AM, Thomas Graf wrote:
> On 12/31/14 at 11:45am, John Fastabend wrote:
>
> Impressive work John, some minor nits below. In general this looks
> great. How large could tables grow? Any risk one of the nested
> attribtues could exceed 16K in size because of a very large parse
> graph? Not a problem if we account for it and allow for jumbo
> attributes.
>
hmm it sounds large to me but maybe if you have an NPU that is trying
to parse into application data it could happen.
What does it take to allow for jumbo attributes?
>> +
>> +/**
>> + * @struct net_flow_header
>> + * @brief defines a match (header/field) an endpoint can use
>> + *
>> + * @uid unique identifier for header
>> + * @field_sz number of fields are in the set
>> + * @fields the set of fields in the net_flow_header
>
> FWIW, name is not documented.
thanks fixed up documentation and spacing for v2.
[...]
>> +#ifndef _UAPI_LINUX_IF_FLOW
>> +#define _UAPI_LINUX_IF_FLOW
>> +
>> +#include <linux/types.h>
>> +#include <linux/netlink.h>
>> +#include <linux/if.h>
>> +
>> +#define NET_FLOW_NAMSIZ 80
>
> Did you consider allocating the memory for names? I don't have a grasp
> for the typical number of net_flow_* instances in memory yet.
>
<100k in the devices I have. Maybe Simon can pitch in what is typical
on the NPUs I'm not sure about them.
Rocker tables can grow as large as needed at the moment.
Allocating the memory may help I'll go ahead and give it a try.
>> +/**
>> + * @struct net_flow_field_ref
>> + * @brief uniquely identify field as header:field tuple
>> + */
>> +struct net_flow_field_ref {
>> + int instance;
>> + int header;
>> + int field;
>> + int mask_type;
>> + int type;
>> + union { /* Are these all the required data types */
>> + __u8 value_u8;
>> + __u16 value_u16;
>> + __u32 value_u32;
>> + __u64 value_u64;
>> + };
>> + union { /* Are these all the required data types */
>> + __u8 mask_u8;
>> + __u16 mask_u16;
>> + __u32 mask_u32;
>> + __u64 mask_u64;
>> + };
>> +};
>
> Does it make sense to write this as follows?
Yes. I'll make this update it helps make it clear value/mask pairs are
needed.
>
> union {
> struct {
> __u8 value_u8;
> __u8 mask_u8;
> };
> struct {
> __u16 value_u16;
> __u16 mask_u16;
> };
> ...
> };
>
>> +#define NET_FLOW_TABLE_EGRESS_ROOT 1
>> +#define NET_FLOW_TABLE_INGRESS_ROOT 2
>
> Tab/space mix.
>
yep fixed.
>> +struct sk_buff *net_flow_build_actions_msg(struct net_flow_action **a,
>> + struct net_device *dev,
>> + u32 portid, int seq, u8 cmd)
>> +{
>> + struct genlmsghdr *hdr;
>> + struct sk_buff *skb;
>> + int err = -ENOBUFS;
>> +
>> + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>
> genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
fixed along with the other cases.
>
>> +static int net_flow_put_table(struct net_device *dev,
>> + struct sk_buff *skb,
>> + struct net_flow_table *t)
>> +{
>> + struct nlattr *matches, *actions;
>> + int i;
>> +
>> + if (nla_put_string(skb, NET_FLOW_TABLE_ATTR_NAME, t->name) ||
>> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_UID, t->uid) ||
>> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_SOURCE, t->source) ||
>> + nla_put_u32(skb, NET_FLOW_TABLE_ATTR_SIZE, t->size))
>> + return -EMSGSIZE;
>> +
>> + matches = nla_nest_start(skb, NET_FLOW_TABLE_ATTR_MATCHES);
>> + if (!matches)
>> + return -EMSGSIZE;
>> +
>> + for (i = 0; t->matches[i].instance; i++)
>> + nla_put(skb, NET_FLOW_FIELD_REF,
>> + sizeof(struct net_flow_field_ref),
>> + &t->matches[i]);
>
> Unhandled nla_put() error
>
thanks.
[...]
>> +static int net_flow_put_headers(struct sk_buff *skb,
>> + struct net_flow_header **headers)
>> +{
>> + struct nlattr *nest, *hdr, *fields;
>> + struct net_flow_header *h;
>> + int i, err;
>> +
>> + nest = nla_nest_start(skb, NET_FLOW_HEADERS);
>> + if (!nest)
>> + return -EMSGSIZE;
>> +
>> + for (i = 0; headers[i]->uid; i++) {
>> + err = -EMSGSIZE;
>> + h = headers[i];
>> +
>> + hdr = nla_nest_start(skb, NET_FLOW_HEADER);
>> + if (!hdr)
>> + goto hdr_put_failure;
>> +
>> + if (nla_put_string(skb, NET_FLOW_HEADER_ATTR_NAME, h->name) ||
>> + nla_put_u32(skb, NET_FLOW_HEADER_ATTR_UID, h->uid))
>> + goto attr_put_failure;
>> +
>> + fields = nla_nest_start(skb, NET_FLOW_HEADER_ATTR_FIELDS);
>> + if (!fields)
>> + goto attr_put_failure;
>
> You can jump to hdr_put_failure right away and get rid of the
> attr_put_failure target as you cancel that nest anyway. You can apply
> this comment to several other places as well if you want.
>
OK so to simplify the error paths we only need to cancel the outer most
nested attribute. I'll do this transformation.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH] Revert "ipw2200: select CFG80211_WEXT"
From: Johannes Berg @ 2015-01-05 18:57 UTC (permalink / raw)
To: Paul Bolle
Cc: Arend van Spriel, Linus Torvalds, Marcel Holtmann,
Stanislav Yakovlev, Kalle Valo, Jiri Kosina, linux-wireless,
Network Development, Linux Kernel Mailing List
In-Reply-To: <1420479510.14308.23.camel@x220>
On Mon, 2015-01-05 at 18:38 +0100, Paul Bolle wrote:
> > ipw2200 is a WEXT driver using some wext functionality (and struct
> > wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that
> > is what makes it confusing.
>
> It doesn't help that I hardly know anything about mac80211, cfg80211 and
> nl80211 (and lib80211 for that matter). To me these are mostly just
> names that end in 80211.
:-)
There isn't really all that much that ipw2x00 is using from cfg80211
though - of note is that it has completely empty ops:
static struct cfg80211_ops libipw_config_ops = { };
IOW - all it does is register with the framework - courtesy of
a3caa99e6c68f. It's practically useless.
> Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
> provided by that symbol that ipw2200 uses directly is
> cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
> that function, something like ipw2100's ipw2100_wx_get_name(). Should be
> trivial to implement (ie, it could take _me_ a day or two).
We could just revert that part of the commit above - or even completely.
However, in theory at least doing *that* would now be a userspace
regression - today you can at least discover the presence of ipw2200
devices with nl80211, even if you can't do anything with them that way.
> But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
> net/wireless/core.c I stumbled on
> #ifdef CONFIG_CFG80211_WEXT
> rdev->wiphy.wext = &cfg80211_wext_handler;
> #endif
I don't think it does - see the note about the ops above. If it did,
it'd have to implement the ops.
> But I net/wireless/wext-core.c I then found
> #ifdef CONFIG_CFG80211_WEXT
> if (dev->ieee80211_ptr && dev->ieee80211_ptr->wiphy)
> handlers = dev->ieee80211_ptr->wiphy->wext;
> #endif
> #ifdef CONFIG_WIRELESS_EXT
> if (dev->wireless_handlers)
> handlers = dev->wireless_handlers;
> #endif
>
> (There's much more to discover about WEXT, of course.) Anyhow, IPW2200
> uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
> ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
> exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
> together.
Well, this was meant as a transition mechanism for drivers. Ultimately,
the way we thought how you'd convert a driver (and how we converted
mac80211) would be to have the wext handlers like for example the scan
ones:
static iw_handler ipw_wx_handlers[] = {
...
IW_HANDLER(SIOCSIWSCAN, ipw_wx_set_scan),
IW_HANDLER(SIOCGIWSCAN, ipw_wx_get_scan),
...
};
Then you could make a patch that uses the cfg80211 APIs for scanning in
the driver -- i.e. implement the cfg80211_ops.scan method, report frames
to the cfg80211 scanning and remove all the ieee->network_list stuff
etc. using the related cfg80211 API (e.g. cfg80211_get_bss() and friends
for getting a network, etc.) And then you'd change the handlers to be
static iw_handler ipw_wx_handlers[] = {
...
IW_HANDLER(SIOCSIWSCAN, cfg80211_wext_siwscan),
IW_HANDLER(SIOCGIWSCAN, cfg80211_wext_giwscan),
...
};
This part would have to be done in a single patch.
Multiple other groups of ioctls could be converted in similar patches,
until at the end you can completely remove ipw_wx_handlers and rely
entirely on cfg80211's wext compatibility.
So far the theory - in practice nobody cared enough to start working on
any of these drivers, let alone actually has the hardware today.
johannes
^ permalink raw reply
* [PATCH v3 4/4] can: kvaser_usb: Add support for the Usbcan-II family
From: Ahmed S. Darwish @ 2015-01-05 18:31 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150105175713.GC6304@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
CAN to USB interfaces sold by the Swedish manufacturer Kvaser are
divided into two major families: 'Leaf', and 'UsbcanII'. From an
Operating System perspective, the firmware of both families behave
in a not too drastically different fashion.
This patch adds support for the UsbcanII family of devices to the
current Kvaser Leaf-only driver.
CAN frames sending, receiving, and error handling paths has been
tested using the dual-channel "Kvaser USBcan II HS/LS" dongle. It
should also work nicely with other products in the same category.
List of new devices supported by this driver update:
- Kvaser USBcan II HS/HS
- Kvaser USBcan II HS/LS
- Kvaser USBcan Rugged ("USBcan Rev B")
- Kvaser Memorator HS/HS
- Kvaser Memorator HS/LS
- Scania VCI2 (if you have the Kvaser logo on top)
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/Kconfig | 8 +-
drivers/net/can/usb/kvaser_usb.c | 618 +++++++++++++++++++++++++++++++--------
2 files changed, 503 insertions(+), 123 deletions(-)
** V2 Changelog:
- Update Kconfig entries
- Use actual number of CAN channels (instead of max) where appropriate
- Rebase over a new set of UsbcanII-independent driver fixes
** V3 Changelog:
- Fix padding for the usbcan_msg_tx_acknowledge command
- Remove kvaser_usb->max_channels and the MAX_NET_DEVICES macro
- Rename commands to CMD_LEAF_xxx and CMD_USBCAN_xxx
- Apply checkpatch.pl suggestions ('net/' comments, multi-line strings, etc.)
diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index a77db919..f6f5500 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -25,7 +25,7 @@ config CAN_KVASER_USB
tristate "Kvaser CAN/USB interface"
---help---
This driver adds support for Kvaser CAN/USB devices like Kvaser
- Leaf Light.
+ Leaf Light and Kvaser USBcan II.
The driver provides support for the following devices:
- Kvaser Leaf Light
@@ -46,6 +46,12 @@ config CAN_KVASER_USB
- Kvaser USBcan R
- Kvaser Leaf Light v2
- Kvaser Mini PCI Express HS
+ - Kvaser USBcan II HS/HS
+ - Kvaser USBcan II HS/LS
+ - Kvaser USBcan Rugged ("USBcan Rev B")
+ - Kvaser Memorator HS/HS
+ - Kvaser Memorator HS/LS
+ - Scania VCI2 (if you have the Kvaser logo on top)
If unsure, say N.
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index cc7bfc0..43b3928 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -6,10 +6,12 @@
* Parts of this driver are based on the following:
* - Kvaser linux leaf driver (version 4.78)
* - CAN driver for esd CAN-USB/2
+ * - Kvaser linux usbcanII driver (version 5.3)
*
* Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
* Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
* Copyright (C) 2012 Olivier Sobrie <olivier@sobrie.be>
+ * Copyright (C) 2015 Valeo Corporation
*/
#include <linux/completion.h>
@@ -21,6 +23,15 @@
#include <linux/can/dev.h>
#include <linux/can/error.h>
+/* Kvaser USB CAN dongles are divided into two major families:
+ * - Leaf: Based on Renesas M32C, running firmware labeled as 'filo'
+ * - UsbcanII: Based on Renesas M16C, running firmware labeled as 'helios'
+ */
+enum kvaser_usb_family {
+ KVASER_LEAF,
+ KVASER_USBCAN,
+};
+
#define MAX_TX_URBS 16
#define MAX_RX_URBS 4
#define START_TIMEOUT 1000 /* msecs */
@@ -30,8 +41,9 @@
#define RX_BUFFER_SIZE 3072
#define CAN_USB_CLOCK 8000000
#define MAX_NET_DEVICES 3
+#define MAX_USBCAN_NET_DEVICES 2
-/* Kvaser USB devices */
+/* Leaf USB devices */
#define KVASER_VENDOR_ID 0x0bfd
#define USB_LEAF_DEVEL_PRODUCT_ID 10
#define USB_LEAF_LITE_PRODUCT_ID 11
@@ -55,6 +67,16 @@
#define USB_CAN_R_PRODUCT_ID 39
#define USB_LEAF_LITE_V2_PRODUCT_ID 288
#define USB_MINI_PCIE_HS_PRODUCT_ID 289
+#define LEAF_PRODUCT_ID(id) (id >= USB_LEAF_DEVEL_PRODUCT_ID && \
+ id <= USB_MINI_PCIE_HS_PRODUCT_ID)
+
+/* USBCANII devices */
+#define USB_USBCAN_REVB_PRODUCT_ID 2
+#define USB_VCI2_PRODUCT_ID 3
+#define USB_USBCAN2_PRODUCT_ID 4
+#define USB_MEMORATOR_PRODUCT_ID 5
+#define USBCAN_PRODUCT_ID(id) (id >= USB_USBCAN_REVB_PRODUCT_ID && \
+ id <= USB_MEMORATOR_PRODUCT_ID)
/* USB devices features */
#define KVASER_HAS_SILENT_MODE BIT(0)
@@ -73,7 +95,7 @@
#define MSG_FLAG_TX_ACK BIT(6)
#define MSG_FLAG_TX_REQUEST BIT(7)
-/* Can states */
+/* Can states (M16C CxSTRH register) */
#define M16C_STATE_BUS_RESET BIT(0)
#define M16C_STATE_BUS_ERROR BIT(4)
#define M16C_STATE_BUS_PASSIVE BIT(5)
@@ -98,7 +120,13 @@
#define CMD_START_CHIP_REPLY 27
#define CMD_STOP_CHIP 28
#define CMD_STOP_CHIP_REPLY 29
-#define CMD_GET_CARD_INFO2 32
+#define CMD_READ_CLOCK 30
+#define CMD_READ_CLOCK_REPLY 31
+
+#define CMD_LEAF_GET_CARD_INFO2 32
+#define CMD_USBCAN_RESET_CLOCK 32
+#define CMD_USBCAN_CLOCK_OVERFLOW_EVENT 33
+
#define CMD_GET_CARD_INFO 34
#define CMD_GET_CARD_INFO_REPLY 35
#define CMD_GET_SOFTWARE_INFO 38
@@ -108,8 +136,9 @@
#define CMD_RESET_ERROR_COUNTER 49
#define CMD_TX_ACKNOWLEDGE 50
#define CMD_CAN_ERROR_EVENT 51
-#define CMD_USB_THROTTLE 77
-#define CMD_LOG_MESSAGE 106
+
+#define CMD_LEAF_USB_THROTTLE 77
+#define CMD_LEAF_LOG_MESSAGE 106
/* error factors */
#define M16C_EF_ACKE BIT(0)
@@ -121,6 +150,14 @@
#define M16C_EF_RCVE BIT(6)
#define M16C_EF_TRE BIT(7)
+/* Only Leaf-based devices can report M16C error factors,
+ * thus define our own error status flags for USBCANII
+ */
+#define USBCAN_ERROR_STATE_NONE 0
+#define USBCAN_ERROR_STATE_TX_ERROR BIT(0)
+#define USBCAN_ERROR_STATE_RX_ERROR BIT(1)
+#define USBCAN_ERROR_STATE_BUSERROR BIT(2)
+
/* bittiming parameters */
#define KVASER_USB_TSEG1_MIN 1
#define KVASER_USB_TSEG1_MAX 16
@@ -137,7 +174,7 @@
#define KVASER_CTRL_MODE_SELFRECEPTION 3
#define KVASER_CTRL_MODE_OFF 4
-/* log message */
+/* Extended CAN identifier flag */
#define KVASER_EXTENDED_FRAME BIT(31)
struct kvaser_msg_simple {
@@ -148,30 +185,55 @@ struct kvaser_msg_simple {
struct kvaser_msg_cardinfo {
u8 tid;
u8 nchannels;
- __le32 serial_number;
- __le32 padding;
+ union {
+ struct {
+ __le32 serial_number;
+ __le32 padding;
+ } __packed leaf0;
+ struct {
+ __le32 serial_number_low;
+ __le32 serial_number_high;
+ } __packed usbcan0;
+ } __packed;
__le32 clock_resolution;
__le32 mfgdate;
u8 ean[8];
u8 hw_revision;
- u8 usb_hs_mode;
- __le16 padding2;
+ union {
+ struct {
+ u8 usb_hs_mode;
+ } __packed leaf1;
+ struct {
+ u8 padding;
+ } __packed usbcan1;
+ } __packed;
+ __le16 padding;
} __packed;
struct kvaser_msg_cardinfo2 {
u8 tid;
- u8 channel;
+ u8 reserved;
u8 pcb_id[24];
__le32 oem_unlock_code;
} __packed;
-struct kvaser_msg_softinfo {
+struct leaf_msg_softinfo {
u8 tid;
- u8 channel;
+ u8 padding0;
__le32 sw_options;
__le32 fw_version;
__le16 max_outstanding_tx;
- __le16 padding[9];
+ __le16 padding1[9];
+} __packed;
+
+struct usbcan_msg_softinfo {
+ u8 tid;
+ u8 fw_name[5];
+ __le16 max_outstanding_tx;
+ u8 padding[6];
+ __le32 fw_version;
+ __le16 checksum;
+ __le16 sw_options;
} __packed;
struct kvaser_msg_busparams {
@@ -188,36 +250,86 @@ struct kvaser_msg_tx_can {
u8 channel;
u8 tid;
u8 msg[14];
- u8 padding;
- u8 flags;
+ union {
+ struct {
+ u8 padding;
+ u8 flags;
+ } __packed leaf;
+ struct {
+ u8 flags;
+ u8 padding;
+ } __packed usbcan;
+ } __packed;
} __packed;
-struct kvaser_msg_rx_can {
+struct kvaser_msg_rx_can_header {
u8 channel;
u8 flag;
+} __packed;
+
+struct leaf_msg_rx_can {
+ u8 channel;
+ u8 flag;
+
__le16 time[3];
u8 msg[14];
} __packed;
-struct kvaser_msg_chip_state_event {
+struct usbcan_msg_rx_can {
+ u8 channel;
+ u8 flag;
+
+ u8 msg[14];
+ __le16 time;
+} __packed;
+
+struct leaf_msg_chip_state_event {
u8 tid;
u8 channel;
+
__le16 time[3];
u8 tx_errors_count;
u8 rx_errors_count;
+
+ u8 status;
+ u8 padding[3];
+} __packed;
+
+struct usbcan_msg_chip_state_event {
+ u8 tid;
+ u8 channel;
+
+ u8 tx_errors_count;
+ u8 rx_errors_count;
+ __le16 time;
+
u8 status;
u8 padding[3];
} __packed;
-struct kvaser_msg_tx_acknowledge {
+struct kvaser_msg_tx_acknowledge_header {
+ u8 channel;
+ u8 tid;
+};
+
+struct leaf_msg_tx_acknowledge {
u8 channel;
u8 tid;
+
__le16 time[3];
u8 flags;
u8 time_offset;
} __packed;
-struct kvaser_msg_error_event {
+struct usbcan_msg_tx_acknowledge {
+ u8 channel;
+ u8 tid;
+
+ __le16 time;
+ __le16 padding;
+} __packed;
+
+struct leaf_msg_error_event {
u8 tid;
u8 flags;
__le16 time[3];
@@ -229,6 +341,18 @@ struct kvaser_msg_error_event {
u8 error_factor;
} __packed;
+struct usbcan_msg_error_event {
+ u8 tid;
+ u8 padding;
+ u8 tx_errors_count_ch0;
+ u8 rx_errors_count_ch0;
+ u8 tx_errors_count_ch1;
+ u8 rx_errors_count_ch1;
+ u8 status_ch0;
+ u8 status_ch1;
+ __le16 time;
+} __packed;
+
struct kvaser_msg_ctrl_mode {
u8 tid;
u8 channel;
@@ -243,7 +367,7 @@ struct kvaser_msg_flush_queue {
u8 padding[3];
} __packed;
-struct kvaser_msg_log_message {
+struct leaf_msg_log_message {
u8 channel;
u8 flags;
__le16 time[3];
@@ -260,19 +384,50 @@ struct kvaser_msg {
struct kvaser_msg_simple simple;
struct kvaser_msg_cardinfo cardinfo;
struct kvaser_msg_cardinfo2 cardinfo2;
- struct kvaser_msg_softinfo softinfo;
struct kvaser_msg_busparams busparams;
+
+ struct kvaser_msg_rx_can_header rx_can_header;
+ struct kvaser_msg_tx_acknowledge_header tx_acknowledge_header;
+
+ union {
+ struct leaf_msg_softinfo softinfo;
+ struct leaf_msg_rx_can rx_can;
+ struct leaf_msg_chip_state_event chip_state_event;
+ struct leaf_msg_tx_acknowledge tx_acknowledge;
+ struct leaf_msg_error_event error_event;
+ struct leaf_msg_log_message log_message;
+ } __packed leaf;
+
+ union {
+ struct usbcan_msg_softinfo softinfo;
+ struct usbcan_msg_rx_can rx_can;
+ struct usbcan_msg_chip_state_event chip_state_event;
+ struct usbcan_msg_tx_acknowledge tx_acknowledge;
+ struct usbcan_msg_error_event error_event;
+ } __packed usbcan;
+
struct kvaser_msg_tx_can tx_can;
- struct kvaser_msg_rx_can rx_can;
- struct kvaser_msg_chip_state_event chip_state_event;
- struct kvaser_msg_tx_acknowledge tx_acknowledge;
- struct kvaser_msg_error_event error_event;
struct kvaser_msg_ctrl_mode ctrl_mode;
struct kvaser_msg_flush_queue flush_queue;
- struct kvaser_msg_log_message log_message;
} u;
} __packed;
+/* Leaf/USBCAN-agnostic summary of an error event.
+ * No M16C error factors for USBCAN-based devices.
+ */
+struct kvaser_error_summary {
+ u8 channel, status, txerr, rxerr;
+ union {
+ struct {
+ u8 error_factor;
+ } leaf;
+ struct {
+ u8 other_ch_status;
+ u8 error_state;
+ } usbcan;
+ };
+};
+
struct kvaser_usb_tx_urb_context {
struct kvaser_usb_net_priv *priv;
u32 echo_index;
@@ -288,6 +443,7 @@ struct kvaser_usb {
u32 fw_version;
unsigned int nchannels;
+ enum kvaser_usb_family family;
bool rxinitdone;
void *rxbuf[MAX_RX_URBS];
@@ -311,6 +467,7 @@ struct kvaser_usb_net_priv {
};
static const struct usb_device_id kvaser_usb_table[] = {
+ /* Leaf family IDs */
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
@@ -360,6 +517,17 @@ static const struct usb_device_id kvaser_usb_table[] = {
.driver_info = KVASER_HAS_TXRX_ERRORS },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_V2_PRODUCT_ID) },
{ USB_DEVICE(KVASER_VENDOR_ID, USB_MINI_PCIE_HS_PRODUCT_ID) },
+
+ /* USBCANII family IDs */
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN2_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN_REVB_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMORATOR_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+ { USB_DEVICE(KVASER_VENDOR_ID, USB_VCI2_PRODUCT_ID),
+ .driver_info = KVASER_HAS_TXRX_ERRORS },
+
{ }
};
MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
@@ -463,7 +631,18 @@ static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
if (err)
return err;
- dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
+ switch (dev->family) {
+ case KVASER_LEAF:
+ dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version);
+ break;
+ case KVASER_USBCAN:
+ dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version);
+ break;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
+ return -EINVAL;
+ }
return 0;
}
@@ -484,6 +663,9 @@ static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
dev->nchannels = msg.u.cardinfo.nchannels;
if (dev->nchannels > MAX_NET_DEVICES)
return -EINVAL;
+ if (dev->family == KVASER_USBCAN &&
+ dev->nchannels > MAX_USBCAN_NET_DEVICES)
+ return -EINVAL;
return 0;
}
@@ -496,8 +678,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
struct kvaser_usb_net_priv *priv;
struct sk_buff *skb;
struct can_frame *cf;
- u8 channel = msg->u.tx_acknowledge.channel;
- u8 tid = msg->u.tx_acknowledge.tid;
+ u8 channel, tid;
+
+ channel = msg->u.tx_acknowledge_header.channel;
+ tid = msg->u.tx_acknowledge_header.tid;
if (channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
@@ -615,37 +799,80 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
priv->tx_contexts[i].echo_index = MAX_TX_URBS;
}
-static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
- const struct kvaser_msg *msg)
+static void kvaser_report_error_event(const struct kvaser_usb *dev,
+ struct kvaser_error_summary *es);
+
+/* Report error to userspace iff the controller's errors counter has
+ * increased, or we're the only channel seeing the bus error state.
+ *
+ * As reported by USBCAN sheets, "the CAN controller has difficulties
+ * to tell whether an error frame arrived on channel 1 or on channel 2."
+ * Thus, error counters are compared with their earlier values to
+ * determine which channel was responsible for the error event.
+ */
+static void usbcan_report_error_if_applicable(const struct kvaser_usb *dev,
+ struct kvaser_error_summary *es)
{
- struct can_frame *cf;
- struct sk_buff *skb;
- struct net_device_stats *stats;
struct kvaser_usb_net_priv *priv;
- unsigned int new_state;
- u8 channel, status, txerr, rxerr, error_factor;
+ int old_tx_err_count, old_rx_err_count, channel, report_error;
+
+ channel = es->channel;
+ if (channel >= dev->nchannels) {
+ dev_err(dev->udev->dev.parent,
+ "Invalid channel number (%d)\n", channel);
+ return;
+ }
+
+ priv = dev->nets[channel];
+ old_tx_err_count = priv->bec.txerr;
+ old_rx_err_count = priv->bec.rxerr;
+
+ report_error = 0;
+ if (es->txerr > old_tx_err_count) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
+ report_error = 1;
+ }
+ if (es->rxerr > old_rx_err_count) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
+ report_error = 1;
+ }
+ if ((es->status & M16C_STATE_BUS_ERROR) &&
+ !(es->usbcan.other_ch_status & M16C_STATE_BUS_ERROR)) {
+ es->usbcan.error_state |= USBCAN_ERROR_STATE_BUSERROR;
+ report_error = 1;
+ }
+
+ if (report_error)
+ kvaser_report_error_event(dev, es);
+}
+
+/* Extract error summary from a Leaf-based device error message */
+static void leaf_extract_error_from_msg(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ struct kvaser_error_summary es = { 0, };
switch (msg->id) {
case CMD_CAN_ERROR_EVENT:
- channel = msg->u.error_event.channel;
- status = msg->u.error_event.status;
- txerr = msg->u.error_event.tx_errors_count;
- rxerr = msg->u.error_event.rx_errors_count;
- error_factor = msg->u.error_event.error_factor;
+ es.channel = msg->u.leaf.error_event.channel;
+ es.status = msg->u.leaf.error_event.status;
+ es.txerr = msg->u.leaf.error_event.tx_errors_count;
+ es.rxerr = msg->u.leaf.error_event.rx_errors_count;
+ es.leaf.error_factor = msg->u.leaf.error_event.error_factor;
break;
- case CMD_LOG_MESSAGE:
- channel = msg->u.log_message.channel;
- status = msg->u.log_message.data[0];
- txerr = msg->u.log_message.data[2];
- rxerr = msg->u.log_message.data[3];
- error_factor = msg->u.log_message.data[1];
+ case CMD_LEAF_LOG_MESSAGE:
+ es.channel = msg->u.leaf.log_message.channel;
+ es.status = msg->u.leaf.log_message.data[0];
+ es.txerr = msg->u.leaf.log_message.data[2];
+ es.rxerr = msg->u.leaf.log_message.data[3];
+ es.leaf.error_factor = msg->u.leaf.log_message.data[1];
break;
case CMD_CHIP_STATE_EVENT:
- channel = msg->u.chip_state_event.channel;
- status = msg->u.chip_state_event.status;
- txerr = msg->u.chip_state_event.tx_errors_count;
- rxerr = msg->u.chip_state_event.rx_errors_count;
- error_factor = 0;
+ es.channel = msg->u.leaf.chip_state_event.channel;
+ es.status = msg->u.leaf.chip_state_event.status;
+ es.txerr = msg->u.leaf.chip_state_event.tx_errors_count;
+ es.rxerr = msg->u.leaf.chip_state_event.rx_errors_count;
+ es.leaf.error_factor = 0;
break;
default:
dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
@@ -653,16 +880,92 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
return;
}
- if (channel >= dev->nchannels) {
+ kvaser_report_error_event(dev, &es);
+}
+
+/* Extract error summary from a USBCANII-based device error message */
+static void usbcan_extract_error_from_msg(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ struct kvaser_error_summary es = { 0, };
+
+ switch (msg->id) {
+ /* Sometimes errors are sent as unsolicited chip state events */
+ case CMD_CHIP_STATE_EVENT:
+ es.channel = msg->u.usbcan.chip_state_event.channel;
+ es.status = msg->u.usbcan.chip_state_event.status;
+ es.txerr = msg->u.usbcan.chip_state_event.tx_errors_count;
+ es.rxerr = msg->u.usbcan.chip_state_event.rx_errors_count;
+ usbcan_report_error_if_applicable(dev, &es);
+ break;
+
+ case CMD_CAN_ERROR_EVENT:
+ es.channel = 0;
+ es.status = msg->u.usbcan.error_event.status_ch0;
+ es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch0;
+ es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch0;
+ es.usbcan.other_ch_status =
+ msg->u.usbcan.error_event.status_ch1;
+ usbcan_report_error_if_applicable(dev, &es);
+
+ /* For error events, the USBCAN firmware does not support
+ * more than 2 channels: ch0, and ch1.
+ */
+ if (dev->nchannels > 1) {
+ es.channel = 1;
+ es.status = msg->u.usbcan.error_event.status_ch1;
+ es.txerr =
+ msg->u.usbcan.error_event.tx_errors_count_ch1;
+ es.rxerr =
+ msg->u.usbcan.error_event.rx_errors_count_ch1;
+ es.usbcan.other_ch_status =
+ msg->u.usbcan.error_event.status_ch0;
+ usbcan_report_error_if_applicable(dev, &es);
+ }
+ break;
+
+ default:
+ dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
+ msg->id);
+ }
+}
+
+static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
+ const struct kvaser_msg *msg)
+{
+ switch (dev->family) {
+ case KVASER_LEAF:
+ leaf_extract_error_from_msg(dev, msg);
+ break;
+ case KVASER_USBCAN:
+ usbcan_extract_error_from_msg(dev, msg);
+ break;
+ default:
dev_err(dev->udev->dev.parent,
- "Invalid channel number (%d)\n", channel);
+ "Invalid device family (%d)\n", dev->family);
return;
}
+}
- priv = dev->nets[channel];
+static void kvaser_report_error_event(const struct kvaser_usb *dev,
+ struct kvaser_error_summary *es)
+{
+ struct can_frame *cf;
+ struct sk_buff *skb;
+ struct net_device_stats *stats;
+ struct kvaser_usb_net_priv *priv;
+ unsigned int new_state;
+
+ if (es->channel >= dev->nchannels) {
+ dev_err(dev->udev->dev.parent,
+ "Invalid channel number (%d)\n", es->channel);
+ return;
+ }
+
+ priv = dev->nets[es->channel];
stats = &priv->netdev->stats;
- if (status & M16C_STATE_BUS_RESET) {
+ if (es->status & M16C_STATE_BUS_RESET) {
kvaser_usb_unlink_tx_urbs(priv);
return;
}
@@ -675,9 +978,9 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
new_state = priv->can.state;
- netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
+ netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
- if (status & M16C_STATE_BUS_OFF) {
+ if (es->status & M16C_STATE_BUS_OFF) {
cf->can_id |= CAN_ERR_BUSOFF;
priv->can.can_stats.bus_off++;
@@ -687,12 +990,12 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
netif_carrier_off(priv->netdev);
new_state = CAN_STATE_BUS_OFF;
- } else if (status & M16C_STATE_BUS_PASSIVE) {
+ } else if (es->status & M16C_STATE_BUS_PASSIVE) {
if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
cf->can_id |= CAN_ERR_CRTL;
- if (txerr || rxerr)
- cf->data[1] = (txerr > rxerr)
+ if (es->txerr || es->rxerr)
+ cf->data[1] = (es->txerr > es->rxerr)
? CAN_ERR_CRTL_TX_PASSIVE
: CAN_ERR_CRTL_RX_PASSIVE;
else
@@ -703,13 +1006,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
}
new_state = CAN_STATE_ERROR_PASSIVE;
- }
-
- if (status == M16C_STATE_BUS_ERROR) {
+ } else if (es->status & M16C_STATE_BUS_ERROR) {
if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
- ((txerr >= 96) || (rxerr >= 96))) {
+ ((es->txerr >= 96) || (es->rxerr >= 96))) {
cf->can_id |= CAN_ERR_CRTL;
- cf->data[1] = (txerr > rxerr)
+ cf->data[1] = (es->txerr > es->rxerr)
? CAN_ERR_CRTL_TX_WARNING
: CAN_ERR_CRTL_RX_WARNING;
@@ -723,7 +1024,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
}
}
- if (!status) {
+ if (!es->status) {
cf->can_id |= CAN_ERR_PROT;
cf->data[2] = CAN_ERR_PROT_ACTIVE;
@@ -739,34 +1040,52 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
priv->can.can_stats.restarts++;
}
- if (error_factor) {
- priv->can.can_stats.bus_error++;
- stats->rx_errors++;
-
- cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
-
- if (error_factor & M16C_EF_ACKE)
- cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
- if (error_factor & M16C_EF_CRCE)
- cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
- CAN_ERR_PROT_LOC_CRC_DEL);
- if (error_factor & M16C_EF_FORME)
- cf->data[2] |= CAN_ERR_PROT_FORM;
- if (error_factor & M16C_EF_STFE)
- cf->data[2] |= CAN_ERR_PROT_STUFF;
- if (error_factor & M16C_EF_BITE0)
- cf->data[2] |= CAN_ERR_PROT_BIT0;
- if (error_factor & M16C_EF_BITE1)
- cf->data[2] |= CAN_ERR_PROT_BIT1;
- if (error_factor & M16C_EF_TRE)
- cf->data[2] |= CAN_ERR_PROT_TX;
+ switch (dev->family) {
+ case KVASER_LEAF:
+ if (es->leaf.error_factor) {
+ priv->can.can_stats.bus_error++;
+ stats->rx_errors++;
+
+ cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
+
+ if (es->leaf.error_factor & M16C_EF_ACKE)
+ cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
+ if (es->leaf.error_factor & M16C_EF_CRCE)
+ cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
+ CAN_ERR_PROT_LOC_CRC_DEL);
+ if (es->leaf.error_factor & M16C_EF_FORME)
+ cf->data[2] |= CAN_ERR_PROT_FORM;
+ if (es->leaf.error_factor & M16C_EF_STFE)
+ cf->data[2] |= CAN_ERR_PROT_STUFF;
+ if (es->leaf.error_factor & M16C_EF_BITE0)
+ cf->data[2] |= CAN_ERR_PROT_BIT0;
+ if (es->leaf.error_factor & M16C_EF_BITE1)
+ cf->data[2] |= CAN_ERR_PROT_BIT1;
+ if (es->leaf.error_factor & M16C_EF_TRE)
+ cf->data[2] |= CAN_ERR_PROT_TX;
+ }
+ break;
+ case KVASER_USBCAN:
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_TX_ERROR)
+ stats->tx_errors++;
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_RX_ERROR)
+ stats->rx_errors++;
+ if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
+ priv->can.can_stats.bus_error++;
+ cf->can_id |= CAN_ERR_BUSERROR;
+ }
+ break;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
+ goto err;
}
- cf->data[6] = txerr;
- cf->data[7] = rxerr;
+ cf->data[6] = es->txerr;
+ cf->data[7] = es->rxerr;
- priv->bec.txerr = txerr;
- priv->bec.rxerr = rxerr;
+ priv->bec.txerr = es->txerr;
+ priv->bec.rxerr = es->rxerr;
priv->can.state = new_state;
@@ -774,6 +1093,11 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
stats->rx_packets++;
stats->rx_bytes += cf->can_dlc;
+
+ return;
+
+err:
+ dev_kfree_skb(skb);
}
static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
@@ -783,16 +1107,16 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
struct sk_buff *skb;
struct net_device_stats *stats = &priv->netdev->stats;
- if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
+ if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
MSG_FLAG_NERR)) {
netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
- msg->u.rx_can.flag);
+ msg->u.rx_can_header.flag);
stats->rx_errors++;
return;
}
- if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
+ if (msg->u.rx_can_header.flag & MSG_FLAG_OVERRUN) {
skb = alloc_can_err_skb(priv->netdev, &cf);
if (!skb) {
stats->rx_dropped++;
@@ -819,7 +1143,8 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
struct can_frame *cf;
struct sk_buff *skb;
struct net_device_stats *stats;
- u8 channel = msg->u.rx_can.channel;
+ u8 channel = msg->u.rx_can_header.channel;
+ const u8 *rx_msg;
if (channel >= dev->nchannels) {
dev_err(dev->udev->dev.parent,
@@ -830,19 +1155,32 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
priv = dev->nets[channel];
stats = &priv->netdev->stats;
- if ((msg->u.rx_can.flag & MSG_FLAG_ERROR_FRAME) &&
- (msg->id == CMD_LOG_MESSAGE)) {
+ if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
+ (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE)) {
kvaser_usb_rx_error(dev, msg);
return;
- } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
- MSG_FLAG_NERR |
- MSG_FLAG_OVERRUN)) {
+ } else if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
+ MSG_FLAG_NERR |
+ MSG_FLAG_OVERRUN)) {
kvaser_usb_rx_can_err(priv, msg);
return;
- } else if (msg->u.rx_can.flag & ~MSG_FLAG_REMOTE_FRAME) {
+ } else if (msg->u.rx_can_header.flag & ~MSG_FLAG_REMOTE_FRAME) {
netdev_warn(priv->netdev,
"Unhandled frame (flags: 0x%02x)",
- msg->u.rx_can.flag);
+ msg->u.rx_can_header.flag);
+ return;
+ }
+
+ switch (dev->family) {
+ case KVASER_LEAF:
+ rx_msg = msg->u.leaf.rx_can.msg;
+ break;
+ case KVASER_USBCAN:
+ rx_msg = msg->u.usbcan.rx_can.msg;
+ break;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
return;
}
@@ -852,38 +1190,37 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
return;
}
- if (msg->id == CMD_LOG_MESSAGE) {
- cf->can_id = le32_to_cpu(msg->u.log_message.id);
+ if (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE) {
+ cf->can_id = le32_to_cpu(msg->u.leaf.log_message.id);
if (cf->can_id & KVASER_EXTENDED_FRAME)
cf->can_id &= CAN_EFF_MASK | CAN_EFF_FLAG;
else
cf->can_id &= CAN_SFF_MASK;
- cf->can_dlc = get_can_dlc(msg->u.log_message.dlc);
+ cf->can_dlc = get_can_dlc(msg->u.leaf.log_message.dlc);
- if (msg->u.log_message.flags & MSG_FLAG_REMOTE_FRAME)
+ if (msg->u.leaf.log_message.flags & MSG_FLAG_REMOTE_FRAME)
cf->can_id |= CAN_RTR_FLAG;
else
- memcpy(cf->data, &msg->u.log_message.data,
+ memcpy(cf->data, &msg->u.leaf.log_message.data,
cf->can_dlc);
} else {
- cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
- (msg->u.rx_can.msg[1] & 0x3f);
+ cf->can_id = ((rx_msg[0] & 0x1f) << 6) | (rx_msg[1] & 0x3f);
if (msg->id == CMD_RX_EXT_MESSAGE) {
cf->can_id <<= 18;
- cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
- ((msg->u.rx_can.msg[3] & 0xff) << 6) |
- (msg->u.rx_can.msg[4] & 0x3f);
+ cf->can_id |= ((rx_msg[2] & 0x0f) << 14) |
+ ((rx_msg[3] & 0xff) << 6) |
+ (rx_msg[4] & 0x3f);
cf->can_id |= CAN_EFF_FLAG;
}
- cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
+ cf->can_dlc = get_can_dlc(rx_msg[5]);
- if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
+ if (msg->u.rx_can_header.flag & MSG_FLAG_REMOTE_FRAME)
cf->can_id |= CAN_RTR_FLAG;
else
- memcpy(cf->data, &msg->u.rx_can.msg[6],
+ memcpy(cf->data, &rx_msg[6],
cf->can_dlc);
}
@@ -947,7 +1284,12 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
case CMD_RX_STD_MESSAGE:
case CMD_RX_EXT_MESSAGE:
- case CMD_LOG_MESSAGE:
+ kvaser_usb_rx_can_msg(dev, msg);
+ break;
+
+ case CMD_LEAF_LOG_MESSAGE:
+ if (dev->family != KVASER_LEAF)
+ goto warn;
kvaser_usb_rx_can_msg(dev, msg);
break;
@@ -960,8 +1302,14 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
kvaser_usb_tx_acknowledge(dev, msg);
break;
+ /* Ignored messages */
+ case CMD_USBCAN_CLOCK_OVERFLOW_EVENT:
+ if (dev->family != KVASER_USBCAN)
+ goto warn;
+ break;
+
default:
- dev_warn(dev->udev->dev.parent,
+warn: dev_warn(dev->udev->dev.parent,
"Unhandled message (%d)\n", msg->id);
break;
}
@@ -1181,7 +1529,7 @@ static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
dev->rxbuf[i],
dev->rxbuf_dma[i]);
- for (i = 0; i < MAX_NET_DEVICES; i++) {
+ for (i = 0; i < dev->nchannels; i++) {
struct kvaser_usb_net_priv *priv = dev->nets[i];
if (priv)
@@ -1289,6 +1637,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
struct kvaser_msg *msg;
int i, err;
int ret = NETDEV_TX_OK;
+ uint8_t *msg_tx_can_flags;
if (can_dropped_invalid_skb(netdev, skb))
return NETDEV_TX_OK;
@@ -1310,9 +1659,23 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
msg = buf;
msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
- msg->u.tx_can.flags = 0;
msg->u.tx_can.channel = priv->channel;
+ switch (dev->family) {
+ case KVASER_LEAF:
+ msg_tx_can_flags = &msg->u.tx_can.leaf.flags;
+ break;
+ case KVASER_USBCAN:
+ msg_tx_can_flags = &msg->u.tx_can.usbcan.flags;
+ break;
+ default:
+ dev_err(dev->udev->dev.parent,
+ "Invalid device family (%d)\n", dev->family);
+ goto releasebuf;
+ }
+
+ *msg_tx_can_flags = 0;
+
if (cf->can_id & CAN_EFF_FLAG) {
msg->id = CMD_TX_EXT_MESSAGE;
msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
@@ -1330,7 +1693,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
if (cf->can_id & CAN_RTR_FLAG)
- msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
+ *msg_tx_can_flags |= MSG_FLAG_REMOTE_FRAME;
for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
@@ -1599,6 +1962,17 @@ static int kvaser_usb_probe(struct usb_interface *intf,
if (!dev)
return -ENOMEM;
+ if (LEAF_PRODUCT_ID(id->idProduct)) {
+ dev->family = KVASER_LEAF;
+ } else if (USBCAN_PRODUCT_ID(id->idProduct)) {
+ dev->family = KVASER_USBCAN;
+ } else {
+ dev_err(&intf->dev,
+ "Product ID (%d) does not belong to any known Kvaser USB family",
+ id->idProduct);
+ return -ENODEV;
+ }
+
err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
if (err) {
dev_err(&intf->dev, "Cannot get usb endpoint(s)");
^ permalink raw reply related
* Re: [CPSW driver] broadcast ethernet pkg will be dropped?
From: Mugunthan V N @ 2015-01-05 18:26 UTC (permalink / raw)
To: Zheng Yi; +Cc: David S. Miller, netdev, Lokesh Vutla
In-Reply-To: <20141229013606.GR970@techyauld.com>
On Monday 29 December 2014 07:06 AM, Zheng Yi wrote:
> Hi Sir
>
> I found your mail address from Linux kernel git repo. I think there
> is a problem in CPSW driver. If it is not your duty to maintain
> those code, could you please tell me the ones who are in charge of
> it?
>
> I found that when a AM3352 board boot, I use a laptop to send ARP
> request to it, the board do not reponse. Even ifconfig do not show
> that there is any pkg sent to the CPSW device. I'm sure that the
> ARP request PKG received by SoC(confirmed by my oscilloscope).
Can you check the hardware statistics counters in CPSW also?
>
> After reading the SoC doc (Tech ref manual) and reading the driver code, i found that:
> 15.3.2.7 Address Lookup Engine (ALE)
> "Broadcast packets will be dropped unless the broadcast address is entered into the table with the super bit set."
>
> and in driver: drivers/net/ether/ti/cpsw_ale.c
>
> int cpsw_ale_add_mcast(struct cpsw_ale *ale, u8 *addr, int port_mask,
> int flags, u16 vid, int mcast_state)
> {
> /* ....... */
>
> cpsw_ale_set_super(ale_entry, (flags & ALE_BLOCKED) ? 1 : 0);
>
> /* ....... */
> }
>
> I have searched all of the calling of that function, and found no
> ALE_BLOCKED flag was set. Especially, in
> cpsw_add_dual_emac_def_ale_entries(), only ALE_VLAN was provided
> for the broadcast address.
>
> After change the flag(brutally), my board can response to the ARP broadcast requests correctly.
>
> Here are some questions I do not found the answer:
> 1. the macro ALE_BLOCKED seems be used uncorrectly.
> If one want the pkg should be "blocked", the "super" bit should be cleared, not be set.
> So, in cpsw_ale_add_mcast(), we should do like this:
> cpsw_ale_set_super(ale_entry, (flags & ALE_BLOCKED) ? 0 : 1);
When Super Bit is set, the packet will never be dropped by any ALE rules
or rate limit. It is made sure that the packet will reach cpu port
irrespective of any rules in the switch. So this is not the correct fix,
need to root cause a bit more.
>
> Do you think the current logic is reversed?
>
> 2. broadcast ethernet pkg from "FF:FF:FF:FF:FF:FF" should be alowed.
> If by default, it desiged to be filtered out, why not add an interface to support
> restore it? I do not found any code that can change filtering the broadcast pkg in runtime.
broadcast is not filtered out, it is always allowed to CPU port in CPSW
driver. Can you dump ALE table entry and check whether something else is
not correct in your setup.
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH] Revert "ipw2200: select CFG80211_WEXT"
From: Arend van Spriel @ 2015-01-05 18:22 UTC (permalink / raw)
To: Paul Bolle
Cc: Linus Torvalds, Marcel Holtmann, Stanislav Yakovlev, Kalle Valo,
Jiri Kosina, linux-wireless, Network Development,
Linux Kernel Mailing List
In-Reply-To: <1420479510.14308.23.camel@x220>
On 01/05/15 18:38, Paul Bolle wrote:
> On Mon, 2015-01-05 at 11:14 +0100, Arend van Spriel wrote:
>> On 01/03/15 23:28, Paul Bolle wrote:
>>> Side note: am I correct in thinking that there's some successor to
>>> CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be
>>> ported to that successor? (ipw2200 hardware appears to be a bit old, so
>>> probably no one would care enough to actually do that.)
>>> net/wireless/kconfig doesn't mention anything like that, so probably I'm
>>> just confused.
>>
>> ipw2200 is a WEXT driver using some wext functionality (and struct
>> wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that
>> is what makes it confusing.
>
> It doesn't help that I hardly know anything about mac80211, cfg80211 and
> nl80211 (and lib80211 for that matter). To me these are mostly just
> names that end in 80211.
Grapjas ;-)
cfg80211 provides thin-layer API for fullmac drivers (running 802.11
stack on the device) and mac80211-based drivers (running 802.11 stack in
kernel).
> Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
> provided by that symbol that ipw2200 uses directly is
> cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
> that function, something like ipw2100's ipw2100_wx_get_name(). Should be
> trivial to implement (ie, it could take _me_ a day or two).
Indeed or even an hour or two.
> But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
> net/wireless/core.c I stumbled on
> #ifdef CONFIG_CFG80211_WEXT
> rdev->wiphy.wext =&cfg80211_wext_handler;
> #endif
This is the "wext compatibility" being enabled for any cfg80211 or
mac80211 based driver.
>
> But I net/wireless/wext-core.c I then found
> #ifdef CONFIG_CFG80211_WEXT
> if (dev->ieee80211_ptr&& dev->ieee80211_ptr->wiphy)
> handlers = dev->ieee80211_ptr->wiphy->wext;
> #endif
wext-core is the WEXT framework and here it extracts WEXT handlers from
a cfg80211/mac80211-based driver that are store in wiphy structure.
> #ifdef CONFIG_WIRELESS_EXT
> if (dev->wireless_handlers)
> handlers = dev->wireless_handlers;
> #endif
Here wext-core extracts WEXT handlers from a WEXT driver. struct
net_device::wireless_handlers is only defined for CONFIG_WIRELESS_EXT.
> (There's much more to discover about WEXT, of course.) Anyhow, IPW2200
> uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
> ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
> exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
> together.
I think ipw2200 is a bit of both worlds indeed adopting the use of
struct wiphy and wiphy_register() call. That seems to suggest it is a
cfg80211 driver, but it does not register any cfg80211 driver callbacks
(see libipw_config_ops in libipw_module.c). So it overrides the WEXT
ioctls because it needs that to interact with the device.
Regards,
Arend
> Thanks,
>
>
> Paul Bolle
>
^ permalink raw reply
* Re: [PATCH] drivers:net:wireless: Add proper locking for the function, b43_op_beacon_set_tim in main.c
From: nick @ 2015-01-05 18:21 UTC (permalink / raw)
To: Larry Finger, Kalle Valo
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
stefano.brivio-hl5o88x/ua9eoWH0uzbU5w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <54AACE00.8060407-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
Larry,
Your right I build tested it. Unfortunately I don't have
the hardware.
Regards Nick
On 2015-01-05 12:46 PM, Larry Finger wrote:
> On 01/05/2015 11:16 AM, nick wrote:
>> Kalle,
>> That's fine. On the other hand I am interested in the
>> reasons why this patch was dropped so I can send in a
>> version 2 of this patch.
>> Regards Nick
>
> Apparently you missed Michael Büsch's comment:
>
> "Thanks for the patch.
>
> However, this does not work. We are in atomic context here.
> Please see the b43-dev mailing list archives for a recent thread about that.
> I'm also pretty sure that this is safe without lock, due to the higher level locks in mac80211."
>
> Did you actually test the patch? If Michael is right, and I trust him on this matter, your patch should lock the system.
>
> Larry
>
^ permalink raw reply
* Re: [PATCH net-next v1 0/7] net: extend ethtool link mode bitmaps to 48 bits
From: David Decotigny @ 2015-01-05 18:16 UTC (permalink / raw)
To: Ben Hutchings
Cc: Maciej Żenczykowski, Amir Vadai, Florian Fainelli,
Linux NetDev, Linux Kernel Mailing List, linux-api,
David S. Miller, Jason Wang, Michael S. Tsirkin, Herbert Xu,
Al Viro, Masatake YAMATO, Xi Wang, Neil Horman, WANG Cong,
Flavio Leitner, Tom Gundersen, Jiri Pirko, Vlad Yasevich,
Eric W. Biederman, Saeed Mahameed, Venkata Duvvuru
In-Reply-To: <1420425003.5686.155.camel@decadent.org.uk>
Thanks Ben, I will send an updated version.
About rejecting high bits in drivers that don't support them: a basic
fix (in a separate patch series) could be something like follows in
ethtool_set_settings callbacks: if (ecmd->advertising_hi) return
-EINVAL; with comments. But I don't find it very nice. Or: allocate a
new net_device::priv_flags bit and ask net/core/ethtool.c to accept
high advertising bits only when this flag is set? Any preference/other
options?
Related: lately, each new class of link modes declared == 4 new bits
allocated. At current pace these 16 new bits buy us only 4 new
classes, ie. a little more than 5 years if I extrapolate from the
recent past. Is the longer term plan to create a new ethtool ioctl
command specialized in link modes with variable length masks? Or to
switch to a brand new netlink interface altogether and take advantage
of that to revisit the link mode reporting/configuration with variable
length masks?
On Sun, Jan 4, 2015 at 6:30 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Mon, 2015-01-05 at 01:34 +0100, Maciej Żenczykowski wrote:
>> >> I can send updates to other drivers, even though it's rather pointless
>> >> to update 1G drivers at this point for example. Please let me know,
>> >> but I'd prefer to do this in follow-up patches outside this first
>> >> patch series.
>> > [...]
>> >
>> > They should be changed to ensure they reject setting any of the high
>> > advertising flags, but it's not urgent.
>>
>> if old drivers advertised a get/set_bits function while new drivers
>> advertised a get/set_new_bits function,
>> you could not updated any old drivers, and simply take care of
>> rejecting invalid bits in core, by calling set_new_bits if provided,
>> if not, rejecting bad bits and calling set_bits if no bad bits were
>> set.
>
> We've never checked that the reserved fields are zero before, and I
> think there are still drivers that don't fully validate the existing 32
> bits. So while I think drivers should fully validate the advertising
> flags, userland generally can't assume they do. And therefore I don't
> think it's worth adding complexity to the ethtool core that only partly
> fixes this.
>
> Ben.
>
> --
> Ben Hutchings
> This sentence contradicts itself - no actually it doesn't.
^ permalink raw reply
* Re: [PATCH v3 05/20] selftests/ftrace: add install target to enable test install
From: Shuah Khan @ 2015-01-05 18:06 UTC (permalink / raw)
To: Steven Rostedt
Cc: mmarek, gregkh, akpm, mingo, davem, keescook, tranmanphong, mpe,
cov, dh.herrmann, hughd, bobby.prani, serge.hallyn, ebiederm,
tim.bird, josh, koct9i, linux-kbuild, linux-kernel, linux-api,
netdev, Shuah Khan
In-Reply-To: <20150102104526.29df5641@gandalf.local.home>
On 01/02/2015 08:45 AM, Steven Rostedt wrote:
> On Wed, 24 Dec 2014 09:27:41 -0700
> Shuah Khan <shuahkh@osg.samsung.com> wrote:
>
>> Add a new make target to enable installing test. This target
>> installs test in the kselftest install location and add to the
>> kselftest script to run the test. Install target can be run
>> only from top level kernel source directory.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>> tools/testing/selftests/ftrace/Makefile | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/ftrace/Makefile b/tools/testing/selftests/ftrace/Makefile
>> index 76cc9f1..7c7cf42 100644
>> --- a/tools/testing/selftests/ftrace/Makefile
>> +++ b/tools/testing/selftests/ftrace/Makefile
>> @@ -1,7 +1,16 @@
>> +TEST_STR = /bin/sh ./ftracetest || echo ftrace selftests: [FAIL]
>
> Is it ok that this removes the quotes around the echo string? I don't
> see anything wrong about it, but I don't know if there's a shell out
> there that will fail due to it.
Right. both sh and bash are fine without the quotes. In this case there
are no variables to interpret, so quotes don't do anything. I might as
well play it safe. I will have to fix a few other tests to address this
comment. Will generate v4s for a few tests in this series.
Thanks,
-- Shuah
>
> Other than than,
>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
>
> -- Steve
>
>
>> +
>> all:
>>
>> +install:
>> +ifdef INSTALL_KSFT_PATH
>> + install ./ftracetest $(INSTALL_KSFT_PATH)
>> + @cp -r test.d $(INSTALL_KSFT_PATH)
>> + echo "$(TEST_STR)" >> $(KSELFTEST)
>> +endif
>> +
>> run_tests:
>> - @/bin/sh ./ftracetest || echo "ftrace selftests: [FAIL]"
>> + @$(TEST_STR)
>>
>> clean:
>> rm -rf logs/*
>
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* [PATCH v3 2/4] can: kvaser_usb: Reset all URB tx contexts upon channel close
From: Ahmed S. Darwish @ 2015-01-05 17:52 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150105174910.GA6304@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Flooding the Kvaser CAN to USB dongle with multiple reads and
writes in very high frequency (*), closing the CAN channel while
all the transmissions are on (#), opening the device again (@),
then sending a small number of packets would make the driver
enter an almost infinite loop of:
[....]
[15959.853988] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853990] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853991] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853993] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853994] kvaser_usb 4-3:1.0 can0: cannot find free context
[15959.853995] kvaser_usb 4-3:1.0 can0: cannot find free context
[....]
_dragging the whole system down_ in the process due to the
excessive logging output.
Initially, this has caused random panics in the kernel due to a
buggy error recovery path. That got fixed in an earlier commit.(%)
This patch aims at solving the root cause. -->
16 tx URBs and contexts are allocated per CAN channel per USB
device. Such URBs are protected by:
a) A simple atomic counter, up to a value of MAX_TX_URBS (16)
b) A flag in each URB context, stating if it's free
c) The fact that ndo_start_xmit calls are themselves protected
by the networking layers higher above
After grabbing one of the tx URBs, if the driver noticed that all
of them are now taken, it stops the netif transmission queue.
Such queue is worken up again only if an acknowedgment was received
from the firmware on one of our earlier-sent frames.
Meanwhile, upon channel close (#), the driver sends a CMD_STOP_CHIP
to the firmware, effectively closing all further communication. In
the high traffic case, the atomic counter remains at MAX_TX_URBS,
and all the URB contexts remain marked as active. While opening
the channel again (@), it cannot send any further frames since no
more free tx URB contexts are available.
Reset all tx URB contexts upon CAN channel close.
(*) 50 parallel instances of `cangen0 -g 0 -ix`
(#) `ifconfig can0 down`
(@) `ifconfig can0 up`
(%) "can: kvaser_usb: Don't free packets when tight on URBs"
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
drivers/net/can/usb/kvaser_usb.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 2e7d513..9accc82 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1246,6 +1246,9 @@ static int kvaser_usb_close(struct net_device *netdev)
if (err)
netdev_warn(netdev, "Cannot stop device, error %d\n", err);
+ /* reset tx contexts */
+ kvaser_usb_unlink_tx_urbs(priv);
+
priv->can.state = CAN_STATE_STOPPED;
close_candev(priv->netdev);
^ permalink raw reply related
* Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Jesse Gross @ 2015-01-05 17:58 UTC (permalink / raw)
To: Fan Du
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, Michael S. Tsirkin,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Wang,
Du, Fan, fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <54AA2912.6090903-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Jan 5, 2015 at 1:02 AM, Fan Du <fengyuleidian0615@gmail.com> wrote:
> 于 2014年12月03日 10:31, Du, Fan 写道:
>
>>
>>
>>> -----Original Message-----
>>> From: Thomas Graf [mailto:tgr@infradead.org] On Behalf Of Thomas Graf
>>> Sent: Wednesday, December 3, 2014 1:42 AM
>>> To: Michael S. Tsirkin
>>> Cc: Du, Fan; 'Jason Wang'; netdev@vger.kernel.org; davem@davemloft.net;
>>> fw@strlen.de; dev@openvswitch.org; jesse@nicira.com; pshelar@nicira.com
>>> Subject: Re: [PATCH net] gso: do GSO for local skb with size bigger than
>>> MTU
>>>
>>> On 12/02/14 at 07:34pm, Michael S. Tsirkin wrote:
>>>>
>>>> On Tue, Dec 02, 2014 at 05:09:27PM +0000, Thomas Graf wrote:
>>>>>
>>>>> On 12/02/14 at 01:48pm, Flavio Leitner wrote:
>>>>>>
>>>>>> What about containers or any other virtualization environment that
>>>>>> doesn't use Virtio?
>>>>>
>>>>>
>>>>> The host can dictate the MTU in that case for both veth or OVS
>>>>> internal which would be primary container plumbing techniques.
>>>>
>>>>
>>>> It typically can't do this easily for VMs with emulated devices:
>>>> real ethernet uses a fixed MTU.
>>>>
>>>> IMHO it's confusing to suggest MTU as a fix for this bug, it's an
>>>> unrelated optimization.
>>>> ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED is the right fix here.
>>>
>>>
>>> PMTU discovery only resolves the issue if an actual IP stack is running
>>> inside the
>>> VM. This may not be the case at all.
>>
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> Some thoughts here:
>>
>> Think otherwise, this is indeed what host stack should forge a
>> ICMP_DEST_UNREACH/ICMP_FRAG_NEEDED
>> message with _inner_ skb network and transport header, do whatever type of
>> encapsulation,
>> and thereafter push such packet upward to Guest/Container, which make them
>> feel, the intermediate node
>> or the peer send such message. PMTU should be expected to work correct.
>> And such behavior should be shared by all other encapsulation tech if they
>> are also suffered.
>
>
> Hi David, Jesse and Thomas
>
> As discussed in here:
> https://www.marc.info/?l=linux-netdev&m=141764712631150&w=4 and
> quotes from Jesse:
> My proposal would be something like this:
> * For L2, reduce the VM MTU to the lowest common denominator on the
> segment.
> * For L3, use path MTU discovery or fragment inner packet (i.e.
> normal routing behavior).
> * As a last resort (such as if using an old version of virtio in the
> guest), fragment the tunnel packet.
>
>
> For L2, it's a administrative action
> For L3, PMTU approach looks better, because once the sender is alerted the
> reduced MTU,
> packet size after encapsulation will not exceed physical MTU, so no
> additional fragments
> efforts needed.
> For "As a last resort... fragment the tunnel packet", the original patch:
> https://www.marc.info/?l=linux-netdev&m=141715655024090&w=4 did the job, but
> seems it's
> not welcomed.
This needs to be properly integrated into IP processing if it is to
work correctly. One of the reasons for only doing path MTU discovery
for L3 is that it operates seamlessly as part of normal operation -
there is no need to forge addresses or potentially generate ICMP when
on an L2 network. However, this ignores the IP handling that is going
on (note that in OVS it is possible for L3 to be implemented as a set
of flows coming from a controller).
It also should not be VXLAN specific or duplicate VXLAN encapsulation
code. As this is happening before encapsulation, the generated ICMP
does not need to be encapsulated either if it is created in the right
location.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH v3 3/4] can: kvaser_usb: Don't send a RESET_CHIP for non-existing channels
From: Ahmed S. Darwish @ 2015-01-05 17:57 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20150105175206.GB6304@linux>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Recent Leaf firmware versions (>= 3.1.557) do not allow to send
commands for non-existing channels. If a command is sent for a
non-existing channel, the firmware crashes.
Reported-by: Christopher Storah <Christopher.Storah@invetech.com.au>
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/can/usb/kvaser_usb.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
** V3 Changelog:
- Update commit log message per Olivier remarks on v2
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 9accc82..cc7bfc0 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1503,6 +1503,10 @@ static int kvaser_usb_init_one(struct usb_interface *intf,
struct kvaser_usb_net_priv *priv;
int i, err;
+ err = kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, channel);
+ if (err)
+ return err;
+
netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
if (!netdev) {
dev_err(&intf->dev, "Cannot alloc candev\n");
@@ -1607,9 +1611,6 @@ static int kvaser_usb_probe(struct usb_interface *intf,
usb_set_intfdata(intf, dev);
- for (i = 0; i < MAX_NET_DEVICES; i++)
- kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
-
err = kvaser_usb_get_software_info(dev);
if (err) {
dev_err(&intf->dev,
^ permalink raw reply related
* [PATCH v3 1/4] can: kvaser_usb: Don't free packets when tight on URBs
From: Ahmed S. Darwish @ 2015-01-05 17:49 UTC (permalink / raw)
To: Olivier Sobrie, Oliver Hartkopp, Wolfgang Grandegger,
Marc Kleine-Budde
Cc: David S. Miller, Paul Gortmaker, Linux-CAN, netdev, LKML
In-Reply-To: <20141223154654.GB6460@vivalin-002>
From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Flooding the Kvaser CAN to USB dongle with multiple reads and
writes in high frequency caused seemingly-random panics in the
kernel.
On further inspection, it seems the driver erroneously freed the
to-be-transmitted packet upon getting tight on URBs and returning
NETDEV_TX_BUSY, leading to invalid memory writes and double frees
at a later point in time.
Note:
Finding no more URBs/transmit-contexts and returning NETDEV_TX_BUSY
is a driver bug in and out of itself: it means that our start/stop
queue flow control is broken.
This patch only fixes the (buggy) error handling code; the root
cause shall be fixed in a later commit.
Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Acked-by: Olivier Sobrie <olivier@sobrie.be>
---
drivers/net/can/usb/kvaser_usb.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
** V3 Changelog:
- checkpatch.pl suggestions ('net/' commenting style)
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 541fb7a..2e7d513 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1294,12 +1294,14 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
if (!urb) {
netdev_err(netdev, "No memory left for URBs\n");
stats->tx_dropped++;
- goto nourbmem;
+ dev_kfree_skb(skb);
+ return NETDEV_TX_OK;
}
buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
if (!buf) {
stats->tx_dropped++;
+ dev_kfree_skb(skb);
goto nobufmem;
}
@@ -1334,6 +1336,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
}
}
+ /* This should never happen; it implies a flow control bug */
if (!context) {
netdev_warn(netdev, "cannot find free context\n");
ret = NETDEV_TX_BUSY;
@@ -1364,9 +1367,6 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
if (unlikely(err)) {
can_free_echo_skb(netdev, context->echo_index);
- skb = NULL; /* set to NULL to avoid double free in
- * dev_kfree_skb(skb) */
-
atomic_dec(&priv->active_tx_urbs);
usb_unanchor_urb(urb);
@@ -1388,8 +1388,6 @@ releasebuf:
kfree(buf);
nobufmem:
usb_free_urb(urb);
-nourbmem:
- dev_kfree_skb(skb);
return ret;
}
^ 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