* 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
* Re: [RFC net-next] ipv6: Use destination address determined by IPVS
From: Julian Anastasov @ 2013-10-17 23:03 UTC (permalink / raw)
To: Simon Horman
Cc: YOSHIFUJI Hideaki / 吉藤英明, lvs-devel,
netdev, Mark Brooks, Phil Oester
In-Reply-To: <1381881751-6719-1-git-send-email-horms@verge.net.au>
Hello,
Here is a solution that should work not only for IPVS.
If the change looks correct I'll send it in a separate message.
[PATCH net] ipv6: always prefer rt6i_gateway if present
From: Julian Anastasov <ja@ssi.bg>
In v3.9 6fd6ce2056de2709 ("ipv6: Do not depend on rt->n in
ip6_finish_output2()." changed the behaviour of ip6_finish_output2()
such that the recently introduced rt6_nexthop() is used
instead of an assigned neighbor.
As rt6_nexthop() prefers rt6i_gateway only for gatewayed
routes this causes a problem for users like IPVS, xt_TEE and
RAW(hdrincl) if they want to use different address for routing
compared to the destination address.
Fix it by considering the rt6i_gateway address in all
cases, so that traffic routed to address on local subnet is
not wrongly diverted to the destination address.
Thanks to Simon Horman and Phil Oester for spotting the
problematic commit.
Reported-by: Phil Oester <kernel@linuxace.com>
Reported-by: Mark Brooks <mark@loadbalancer.org>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
Please review for possible side effects when using
rt6i_gateway without RTF_GATEWAY!
include/net/ip6_route.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index f525e70..481404a 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -196,7 +196,7 @@ static inline int ip6_skb_dst_mtu(struct sk_buff *skb)
static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt, struct in6_addr *dest)
{
- if (rt->rt6i_flags & RTF_GATEWAY)
+ if (rt->rt6i_flags & RTF_GATEWAY || !ipv6_addr_any(&rt->rt6i_gateway))
return &rt->rt6i_gateway;
return dest;
}
--
1.8.3.1
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: refactor sk_page_frag_refill()
From: Eric Dumazet @ 2013-10-17 23:13 UTC (permalink / raw)
To: David Miller
Cc: mwdalton, dborkman, netdev, ffusco, ycheng, ncardwell,
digitaleric
In-Reply-To: <20131017.154625.1687353288700164892.davem@davemloft.net>
On Thu, 2013-10-17 at 15:46 -0400, David Miller wrote:
> Please rename this to something like "skb_page_frag_refill" so that it
> is clear that this is a networking interface.
Sure will do, thanks !
^ permalink raw reply
* [PATCH v3 net-next] net: refactor sk_page_frag_refill()
From: Eric Dumazet @ 2013-10-17 23:27 UTC (permalink / raw)
To: Michael Dalton
Cc: Daniel Borkmann, davem, netdev, Francesco Fusco, ycheng,
Neal Cardwell, Eric Northup
In-Reply-To: <1381639591.3392.31.camel@edumazet-glaptop.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
While working on virtio_net new allocation strategy to increase
payload/truesize ratio, we found that refactoring sk_page_frag_refill()
was needed.
This patch splits sk_page_frag_refill() into two parts, adding
skb_page_frag_refill() which can be used without a socket.
While we are at it, add a minimum frag size of 32 for
sk_page_frag_refill()
Michael will either use netdev_alloc_frag() from softirq context,
or skb_page_frag_refill() from process context in refill_work()
(GFP_KERNEL allocations)
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Michael Dalton <mwdalton@google.com>
---
v3: page_frag_refill() -> skb_page_frag_refill()
v2: fix a off-by one error
include/linux/skbuff.h | 2 ++
net/core/sock.c | 27 +++++++++++++++++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1cd32f9..ba74474 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2062,6 +2062,8 @@ static inline void skb_frag_set_page(struct sk_buff *skb, int f,
__skb_frag_set_page(&skb_shinfo(skb)->frags[f], page);
}
+bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t prio);
+
/**
* skb_frag_dma_map - maps a paged fragment via the DMA API
* @dev: the device to map the fragment to
diff --git a/net/core/sock.c b/net/core/sock.c
index fd6afa2..440afdc 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1847,7 +1847,17 @@ EXPORT_SYMBOL(sock_alloc_send_skb);
/* On 32bit arches, an skb frag is limited to 2^15 */
#define SKB_FRAG_PAGE_ORDER get_order(32768)
-bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
+/**
+ * skb_page_frag_refill - check that a page_frag contains enough room
+ * @sz: minimum size of the fragment we want to get
+ * @pfrag: pointer to page_frag
+ * @prio: priority for memory allocation
+ *
+ * Note: While this allocator tries to use high order pages, there is
+ * no guarantee that allocations succeed. Therefore, @sz MUST be
+ * less or equal than PAGE_SIZE.
+ */
+bool skb_page_frag_refill(unsigned int sz, struct page_frag *pfrag, gfp_t prio)
{
int order;
@@ -1856,16 +1866,16 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
pfrag->offset = 0;
return true;
}
- if (pfrag->offset < pfrag->size)
+ if (pfrag->offset + sz <= pfrag->size)
return true;
put_page(pfrag->page);
}
/* We restrict high order allocations to users that can afford to wait */
- order = (sk->sk_allocation & __GFP_WAIT) ? SKB_FRAG_PAGE_ORDER : 0;
+ order = (prio & __GFP_WAIT) ? SKB_FRAG_PAGE_ORDER : 0;
do {
- gfp_t gfp = sk->sk_allocation;
+ gfp_t gfp = prio;
if (order)
gfp |= __GFP_COMP | __GFP_NOWARN;
@@ -1877,6 +1887,15 @@ bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
}
} while (--order >= 0);
+ return false;
+}
+EXPORT_SYMBOL(skb_page_frag_refill);
+
+bool sk_page_frag_refill(struct sock *sk, struct page_frag *pfrag)
+{
+ if (likely(skb_page_frag_refill(32U, pfrag, sk->sk_allocation)))
+ return true;
+
sk_enter_memory_pressure(sk);
sk_stream_moderate_sndbuf(sk);
return false;
^ permalink raw reply related
* Re: [PATCH net-next v3 3/9] static_key: WARN on usage before jump_label_init was called
From: Hannes Frederic Sowa @ 2013-10-18 0:01 UTC (permalink / raw)
To: Steven Rostedt; +Cc: netdev, linux-kernel, Peter Zijlstra, Andi Kleen
In-Reply-To: <20131017171940.173bab09@gandalf.local.home>
On Thu, Oct 17, 2013 at 05:19:40PM -0400, Steven Rostedt wrote:
> 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?
Ok. :)
I'll move the description from the changelog here and write something
in a few days (hope to get more feedback on the other parts, especially
net_get_random_once).
Does that also mean you are in principle ok with the patch weakening
the check in arch/x86/jump_label.c?
Thanks for the review,
Hannes
^ permalink raw reply
* [PATCH 00/15] net: ethernet: remove unnecessary pci_set_drvdata() part 1
From: Jingoo Han @ 2013-10-18 0:16 UTC (permalink / raw)
To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han'
Since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
(device-core: Ensure drvdata = NULL when no driver is bound),
the driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
---
drivers/net/ethernet/3com/typhoon.c | 1 -
drivers/net/ethernet/8390/ne2k-pci.c | 3 ---
drivers/net/ethernet/adaptec/starfire.c | 2 --
drivers/net/ethernet/amd/amd8111e.c | 2 --
drivers/net/ethernet/amd/pcnet32.c | 1 -
drivers/net/ethernet/atheros/alx/main.c | 1 -
drivers/net/ethernet/broadcom/bnx2.c | 3 ---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 3 ---
drivers/net/ethernet/broadcom/tg3.c | 2 --
drivers/net/ethernet/brocade/bna/bnad.c | 1 -
drivers/net/ethernet/chelsio/cxgb/cxgb2.c | 2 --
drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 2 --
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 --
drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 3 ---
drivers/net/ethernet/cisco/enic/enic_main.c | 2 --
15 files changed, 30 deletions(-)
^ permalink raw reply
* [PATCH 01/15] net: typhoon: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:17 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'David Dillow'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Acked-by: David Dillow <dave@thedillows.org>
---
drivers/net/ethernet/3com/typhoon.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index 144942f6..465cc71 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2525,7 +2525,6 @@ typhoon_remove_one(struct pci_dev *pdev)
pci_release_regions(pdev);
pci_clear_mwi(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
free_netdev(dev);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 02/15] net: 8390: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:18 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Paul Gortmaker'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/8390/ne2k-pci.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/8390/ne2k-pci.c b/drivers/net/ethernet/8390/ne2k-pci.c
index 9220108..fc14a85 100644
--- a/drivers/net/ethernet/8390/ne2k-pci.c
+++ b/drivers/net/ethernet/8390/ne2k-pci.c
@@ -389,9 +389,7 @@ err_out_free_netdev:
free_netdev (dev);
err_out_free_res:
release_region (ioaddr, NE_IO_EXTENT);
- pci_set_drvdata (pdev, NULL);
return -ENODEV;
-
}
/*
@@ -655,7 +653,6 @@ static void ne2k_pci_remove_one(struct pci_dev *pdev)
release_region(dev->base_addr, NE_IO_EXTENT);
free_netdev(dev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
#ifdef CONFIG_PM
--
1.7.10.4
^ permalink raw reply related
* [PATCH 03/15] net: starfire: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:18 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Ion Badulescu'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/adaptec/starfire.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index 8b04bfc..171d73c 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -835,7 +835,6 @@ static int starfire_init_one(struct pci_dev *pdev,
return 0;
err_out_cleardev:
- pci_set_drvdata(pdev, NULL);
iounmap(base);
err_out_free_res:
pci_release_regions (pdev);
@@ -2012,7 +2011,6 @@ static void starfire_remove_one(struct pci_dev *pdev)
iounmap(np->base);
pci_release_regions(pdev);
- pci_set_drvdata(pdev, NULL);
free_netdev(dev); /* Will also free np!! */
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 04/15] net: pcnet32: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:19 UTC (permalink / raw)
To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han', 'Don Fry'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Acked-by: Don Fry <pcnet32@frontier.com>
---
drivers/net/ethernet/amd/pcnet32.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index bd4e640..38492e0 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -2818,7 +2818,6 @@ static void pcnet32_remove_one(struct pci_dev *pdev)
lp->init_block, lp->init_dma_addr);
free_netdev(dev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 05/15] net: amd8111e: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:19 UTC (permalink / raw)
To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/amd/amd8111e.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/amd/amd8111e.c b/drivers/net/ethernet/amd/amd8111e.c
index 1b1429d..d042511 100644
--- a/drivers/net/ethernet/amd/amd8111e.c
+++ b/drivers/net/ethernet/amd/amd8111e.c
@@ -1711,7 +1711,6 @@ static void amd8111e_remove_one(struct pci_dev *pdev)
free_netdev(dev);
pci_release_regions(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
}
}
static void amd8111e_config_ipg(struct net_device* dev)
@@ -1967,7 +1966,6 @@ err_free_reg:
err_disable_pdev:
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
return err;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 06/15] net: alx: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-10-18 0:20 UTC (permalink / raw)
To: 'David S. Miller'
Cc: netdev, 'Jingoo Han', 'Jay Cliburn',
'Chris Snook'
In-Reply-To: <00ae01cecb97$44d45770$ce7d0650$%han@samsung.com>
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/net/ethernet/atheros/alx/main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index fc95b23..5aa5e81 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1367,7 +1367,6 @@ static void alx_remove(struct pci_dev *pdev)
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
- pci_set_drvdata(pdev, NULL);
free_netdev(alx->dev);
}
--
1.7.10.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