* Re: [patch -resend] 9p: fix min_t() casting in p9pdu_vwritef()
From: walter harms @ 2012-06-27 10:19 UTC (permalink / raw)
To: Dan Carpenter
Cc: Eric Van Hensbergen, David S. Miller, Aneesh Kumar K.V, netdev,
kernel-janitors
In-Reply-To: <20120627090141.GF31212@elgon.mountain>
Am 27.06.2012 11:01, schrieb Dan Carpenter:
> I don't think we're actually likely to hit this limit but if we do
> then the comparison should be done as size_t. The original code
> is equivalent to:
> len = strlen(sptr) % USHRT_MAX;
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I was told this patch "has already made it upstream via the v9fs pull."
> but it must have been dropped accidentally. Originally sent on Sat,
> Jan 15, 2011.
>
> diff --git a/net/9p/protocol.c b/net/9p/protocol.c
> index 9ee48cb..3d33ecf 100644
> --- a/net/9p/protocol.c
> +++ b/net/9p/protocol.c
> @@ -368,7 +368,7 @@ p9pdu_vwritef(struct p9_fcall *pdu, int proto_version, const char *fmt,
> const char *sptr = va_arg(ap, const char *);
> uint16_t len = 0;
> if (sptr)
> - len = min_t(uint16_t, strlen(sptr),
> + len = min_t(size_t, strlen(sptr),
> USHRT_MAX);
>
> errcode = p9pdu_writef(pdu, proto_version,
this will result in
uint16_t = size_t
i would expect compilers to complains since uint16 < size_t (most times). In this special case
it seems more easy write it. also ushort seems ambitious since uint16_t need not to be ushort.
so my idea would look like this:
len=strlen
if (len>65535) len=65535;
p9pdu_writef(...,(unint16_t)len);
just my 2 cents,
wh
^ permalink raw reply
* [PATCH net-next] net: skb_free_datagram_locked() doesnt drop all packets
From: Eric Dumazet @ 2012-06-27 10:23 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
dropwatch wrongly diagnose all received UDP packets as drops.
This patch removes trace_kfree_skb() done in skb_free_datagram_locked().
Locations calling skb_free_datagram_locked() should do it on their own.
As a result, drops are accounted on the right function.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/core/datagram.c | 1 -
net/ipv4/udp.c | 5 ++++-
net/ipv6/udp.c | 8 +++++---
net/sunrpc/svcsock.c | 12 ++++++------
4 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/net/core/datagram.c b/net/core/datagram.c
index ae6acf6..0337e2b 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -248,7 +248,6 @@ void skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb)
unlock_sock_fast(sk, slow);
/* skb is now orphaned, can be freed outside of locked section */
- trace_kfree_skb(skb, skb_free_datagram_locked);
__kfree_skb(skb);
}
EXPORT_SYMBOL(skb_free_datagram_locked);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index db017ef..ee37d47 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -108,6 +108,7 @@
#include <net/xfrm.h>
#include <trace/events/udp.h>
#include <linux/static_key.h>
+#include <trace/events/skb.h>
#include "udp_impl.h"
struct udp_table udp_table __read_mostly;
@@ -1220,8 +1221,10 @@ try_again:
goto csum_copy_err;
}
- if (err)
+ if (unlikely(err)) {
+ trace_kfree_skb(skb, udp_recvmsg);
goto out_free;
+ }
if (!peeked)
UDP_INC_STATS_USER(sock_net(sk),
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 051ad48..1ecd102 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -48,6 +48,7 @@
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <trace/events/skb.h>
#include "udp_impl.h"
int ipv6_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2)
@@ -385,15 +386,16 @@ try_again:
if (skb_csum_unnecessary(skb))
err = skb_copy_datagram_iovec(skb, sizeof(struct udphdr),
- msg->msg_iov, copied );
+ msg->msg_iov, copied);
else {
err = skb_copy_and_csum_datagram_iovec(skb, sizeof(struct udphdr), msg->msg_iov);
if (err == -EINVAL)
goto csum_copy_err;
}
- if (err)
+ if (unlikely(err)) {
+ trace_kfree_skb(skb, udpv6_recvmsg);
goto out_free;
-
+ }
if (!peeked) {
if (is_udp4)
UDP_INC_STATS_USER(sock_net(sk),
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index a6de09d..18bc130 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -43,6 +43,7 @@
#include <net/tcp_states.h>
#include <asm/uaccess.h>
#include <asm/ioctls.h>
+#include <trace/events/skb.h>
#include <linux/sunrpc/types.h>
#include <linux/sunrpc/clnt.h>
@@ -619,6 +620,8 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
if (!svc_udp_get_dest_address(rqstp, cmh)) {
net_warn_ratelimited("svc: received unknown control message %d/%d; dropping RPC reply datagram\n",
cmh->cmsg_level, cmh->cmsg_type);
+out_free:
+ trace_kfree_skb(skb, svc_udp_recvfrom);
skb_free_datagram_locked(svsk->sk_sk, skb);
return 0;
}
@@ -630,8 +633,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
if (csum_partial_copy_to_xdr(&rqstp->rq_arg, skb)) {
local_bh_enable();
/* checksum error */
- skb_free_datagram_locked(svsk->sk_sk, skb);
- return 0;
+ goto out_free;
}
local_bh_enable();
skb_free_datagram_locked(svsk->sk_sk, skb);
@@ -640,10 +642,8 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
rqstp->rq_arg.head[0].iov_base = skb->data +
sizeof(struct udphdr);
rqstp->rq_arg.head[0].iov_len = len;
- if (skb_checksum_complete(skb)) {
- skb_free_datagram_locked(svsk->sk_sk, skb);
- return 0;
- }
+ if (skb_checksum_complete(skb))
+ goto out_free;
rqstp->rq_xprt_ctxt = skb;
}
^ permalink raw reply related
* Re: [PATCH] sctp: be mroe restrictive in transport selection on bundled sacks
From: Neil Horman @ 2012-06-27 10:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev, vyasevich, linux-sctp
In-Reply-To: <20120626.210504.657326352932517334.davem@davemloft.net>
On Tue, Jun 26, 2012 at 09:05:04PM -0700, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Tue, 26 Jun 2012 16:31:44 -0400
>
> > @@ -240,15 +240,20 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,
> > */
> > if (sctp_chunk_is_data(chunk) && !pkt->has_sack &&
> > !pkt->has_cookie_echo) {
> > - struct sctp_association *asoc;
> > struct timer_list *timer;
> > - asoc = pkt->transport->asoc;
> > + struct sctp_association *asoc = pkt->transport->asoc;
> > + struct sctp_transport *trans;
> > +
> > timer = &asoc->timers[SCTP_EVENT_TIMEOUT_SACK];
> >
> > /* If the SACK timer is running, we have a pending SACK */
> > if (timer_pending(timer)) {
> > struct sctp_chunk *sack;
> > asoc->a_rwnd = asoc->rwnd;
> > +
> > + if (chunk->transport && !chunk->transport->moved_ctsn)
> > + return retval;
> > +
> > sack = sctp_make_sack(asoc);
> > if (sack) {
> > retval = sctp_packet_append_chunk(pkt, sack);
>
> The new local variable 'trans' seems to be unused.
>
Crap, thank you Dave, that was a holdover from an initial pass I had made in
writing this. I'll repost with that removed once Vald has a chance to look this
over
Neil
^ permalink raw reply
* Re: [patch -resend] 9p: fix min_t() casting in p9pdu_vwritef()
From: Dan Carpenter @ 2012-06-27 10:36 UTC (permalink / raw)
To: walter harms
Cc: Eric Van Hensbergen, David S. Miller, Aneesh Kumar K.V, netdev,
kernel-janitors
In-Reply-To: <4FEADE2E.90005@bfs.de>
On Wed, Jun 27, 2012 at 12:19:26PM +0200, walter harms wrote:
>
>
> Am 27.06.2012 11:01, schrieb Dan Carpenter:
> > I don't think we're actually likely to hit this limit but if we do
> > then the comparison should be done as size_t. The original code
> > is equivalent to:
> > len = strlen(sptr) % USHRT_MAX;
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > I was told this patch "has already made it upstream via the v9fs pull."
> > but it must have been dropped accidentally. Originally sent on Sat,
> > Jan 15, 2011.
> >
> > diff --git a/net/9p/protocol.c b/net/9p/protocol.c
> > index 9ee48cb..3d33ecf 100644
> > --- a/net/9p/protocol.c
> > +++ b/net/9p/protocol.c
> > @@ -368,7 +368,7 @@ p9pdu_vwritef(struct p9_fcall *pdu, int proto_version, const char *fmt,
> > const char *sptr = va_arg(ap, const char *);
> > uint16_t len = 0;
> > if (sptr)
> > - len = min_t(uint16_t, strlen(sptr),
> > + len = min_t(size_t, strlen(sptr),
> > USHRT_MAX);
> >
> > errcode = p9pdu_writef(pdu, proto_version,
>
> this will result in
> uint16_t = size_t
> i would expect compilers to complains since uint16 < size_t
> (most times). In this special case it seems more easy write it.
> also ushort seems ambitious since uint16_t need not to be
> ushort. so my idea would look like this:
>
> len=strlen
> if (len>65535) len=65535;
> p9pdu_writef(...,(unint16_t)len);
>
No. I'm sorry, what you're saying is complete nonsense. The whole
point of min_t() is that you can cast to both sides to what you want
before you do the compare.
Obviously I wouldn't submit a patch that introduces a compile
warning. :/
regards,
dan carpenter
^ permalink raw reply
* [Xen-devel] [PATCH 1/1] xen/netback: only non-freed SKB is queued into tx_queue
From: annie.li @ 2012-06-27 10:46 UTC (permalink / raw)
To: xen-devel, netdev, davem, Ian.Campbell, konrad.wilk
Cc: kurt.hackel, annie.li, Annie Li
From: Annie Li <Annie.li@oracle.com>
After SKB is queued into tx_queue, it will be freed if request_gop is NULL.
However, no dequeue action is called in this situation, it is likely that
tx_queue constains freed SKB. This patch should fix this issue, and it is
based on 3.5.0-rc4+.
This issue is found through code inspection, no bug is seen with it currently.
I run netperf test for several hours, and no network regression was found.
Signed-off-by: Annie Li <annie.li@oracle.com>
---
drivers/net/xen-netback/netback.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index f4a6fca..682633b 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1363,8 +1363,6 @@ static unsigned xen_netbk_tx_build_gops(struct xen_netbk *netbk)
INVALID_PENDING_IDX);
}
- __skb_queue_tail(&netbk->tx_queue, skb);
-
netbk->pending_cons++;
request_gop = xen_netbk_get_requests(netbk, vif,
@@ -1376,6 +1374,8 @@ static unsigned xen_netbk_tx_build_gops(struct xen_netbk *netbk)
}
gop = request_gop;
+ __skb_queue_tail(&netbk->tx_queue, skb);
+
vif->tx.req_cons = idx;
xen_netbk_check_rx_xenvif(vif);
--
1.7.6.5
^ permalink raw reply related
* Re: [PATCH 2/18] gdm72xx: Move away from NLMSG_PUT().
From: Javier Martinez Canillas @ 2012-06-27 11:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120626.220206.1076042544693422783.davem@davemloft.net>
On Wed, Jun 27, 2012 at 7:02 AM, David Miller <davem@davemloft.net> wrote:
>
> And use nlmsg_data() while we're here too.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> drivers/staging/gdm72xx/netlink_k.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/gdm72xx/netlink_k.c b/drivers/staging/gdm72xx/netlink_k.c
> index 292af0f..d0cb48a 100644
> --- a/drivers/staging/gdm72xx/netlink_k.c
> +++ b/drivers/staging/gdm72xx/netlink_k.c
> @@ -127,8 +127,12 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
> }
>
> seq++;
> - nlh = NLMSG_PUT(skb, 0, seq, type, len);
> - memcpy(NLMSG_DATA(nlh), msg, len);
> + nlh = nlmsg_put(skb, 0, seq, type, len, 0);
> + if (!nlh) {
> + kfree_skb(skb);
> + return -EMSGSIZE;
> + }
> + memcpy(nlmsg_data(nlh), msg, len);
>
> NETLINK_CB(skb).pid = 0;
> NETLINK_CB(skb).dst_group = 0;
> @@ -144,7 +148,5 @@ int netlink_send(struct sock *sock, int group, u16 type, void *msg, int len)
> }
> ret = 0;
> }
> -
> -nlmsg_failure:
> return ret;
> }
> --
> 1.7.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi David,
This also solves an skb memory leak since the sk_buff was allocated
before the NLMSG_PUT() macro and not freed after jumping to the
nlmsg_failure label. I posted a similar patch before:
http://www.spinics.net/lists/linux-driver-devel/msg25756.html
and is already on linux-next with commitid:
2da049bd5f9b0dbd688519fdb6688a4895fe8395 staging: gdm72xx: fix an skb
memory leak
Best regards,
Javier
^ permalink raw reply
* LOCKDEP complaints in l2tp_xmit_skb()
From: Tom Parkin @ 2012-06-27 11:11 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 9304 bytes --]
In testing L2TP ethernet pseudowires I have observed some complaints
from lockdep due to circular/recursive locking in l2tp_xmit_skb().
I'm testing the -net tree, which includes Eric's recent patches to
squash another lockdep error by converting l2tp to LLTX. Git hash
d7ffde35e31a811.
My test setup consists of two AMD64 boxes, both running 32bit kernels.
One box is SMP, the other UP. My test procedure consists of creating
an L2TP tunnel containing N ethernet pseudowires. I then run N iperf
sessions across the N pseudowires. The simplest configuration is:
[On HOST A]
ip l2tp add tunnel \
tunnel_id 1 \
peer_tunnel_id 1 \
local <HOST A ip> \
remote <HOST B ip> \
udp_sport 9999 \
udp_dport 9999
ip add session \
tunnel_id 1 \
session_id 1 \
peer_session_id 1
ip addr add 172.16.0.1 \
peer 172.16.0.2/24 \
broadcast 172.16.0.255 \
dev l2tpeth0
ip link set l2tpeth0 up
iperf -s -B 172.16.0.1
[On HOST B]
ip l2tp add tunnel \
tunnel_id 1 \
peer_tunnel_id 1 \
local <HOST B ip> \
remote <HOST A ip> \
udp_sport 9999 \
udp_dport 9999
ip add session \
tunnel_id 1 \
session_id 1 \
peer_session_id 1
ip addr add 172.16.0.2 \
peer 172.16.0.1/24 \
broadcast 172.16.0.255 \
dev l2tpeth0
ip link set l2tpeth0 up
iperf -c 172.16.0.1
If I run four concurrent iperf sessions across four pseudowires I
see lockdep complaints on both SMP and UP boxes.
Lockdep output for the AMD64 SMP machine:
======================================================
[ INFO: possible circular locking dependency detected ]
3.5.0-rc2-net-lockdep-u64-sync-006-+ #2 Not tainted
-------------------------------------------------------
swapper/1/0 is trying to acquire lock:
(slock-AF_INET){+.-...}, at: [<f85f5bff>] l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
but task is already holding lock:
(&(&sch->busylock)->rlock){+.-...}, at: [<c14fb1b2>] dev_queue_xmit+0xb42/0xbd0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&(&sch->busylock)->rlock){+.-...}:
[<c10a8b48>] lock_acquire+0x88/0x120
[<c16157bb>] _raw_spin_lock+0x3b/0x70
[<c1535cf8>] __inet_hash_nolisten+0xb8/0x140
[<c1536b77>] __inet_hash_connect+0x267/0x2c0
[<c1536c10>] inet_hash_connect+0x40/0x50
[<c154e4d4>] tcp_v4_connect+0x2c4/0x510
[<c156293f>] inet_stream_connect+0x1ff/0x380
[<c14e30c1>] sys_connect+0xc1/0xe0
[<c14e3d13>] sys_socketcall+0xe3/0x2e0
[<c161d89f>] sysenter_do_call+0x12/0x38
-> #0 (slock-AF_INET){+.-...}:
[<c10a78cc>] __lock_acquire+0xaec/0x17d0
[<c10a8b48>] lock_acquire+0x88/0x120
[<c16157bb>] _raw_spin_lock+0x3b/0x70
[<f85f5bff>] l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
[<f851432d>] l2tp_eth_dev_xmit+0x2d/0x40 [l2tp_eth]
[<c14fa32f>] dev_hard_start_xmit+0x49f/0x7e0
[<c1515819>] sch_direct_xmit+0xa9/0x250
[<c14fa835>] dev_queue_xmit+0x1c5/0xbd0
[<c159442c>] ip6_finish_output2+0x11c/0x620
[<c159813f>] ip6_finish_output+0x7f/0x1e0
[<c15982ea>] ip6_output+0x4a/0x1f0
[<c15bbddc>] mld_sendpack+0x21c/0x530
[<c15bc817>] mld_ifc_timer_expire+0x187/0x260
[<c1055d10>] run_timer_softirq+0x140/0x370
[<c104da27>] __do_softirq+0x97/0x1f0
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&(&sch->busylock)->rlock);
lock(slock-AF_INET);
lock(&(&sch->busylock)->rlock);
lock(slock-AF_INET);
*** DEADLOCK ***
5 locks held by swapper/1/0:
#0: (&idev->mc_ifc_timer){+.-...}, at: [<c1055c88>] run_timer_softirq+0xb8/0x370
#1: (rcu_read_lock){.+.+..}, at: [<c15bbbc0>] mld_sendpack+0x0/0x530
#2: (rcu_read_lock){.+.+..}, at: [<c159434f>] ip6_finish_output2+0x3f/0x620
#3: (rcu_read_lock_bh){.+....}, at: [<c14fa670>] dev_queue_xmit+0x0/0xbd0
#4: (&(&sch->busylock)->rlock){+.-...}, at: [<c14fb1b2>] dev_queue_xmit+0xb42/0xbd0
stack backtrace:
Pid: 0, comm: swapper/1 Not tainted 3.5.0-rc2-net-lockdep-u64-sync-006-+ #2
Call Trace:
[<c160b540>] print_circular_bug+0x1b4/0x1be
[<c10a78cc>] __lock_acquire+0xaec/0x17d0
[<c10a8b48>] lock_acquire+0x88/0x120
[<f85f5bff>] ? l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
[<c16157bb>] _raw_spin_lock+0x3b/0x70
[<f85f5bff>] ? l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
[<f85f5bff>] l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
[<f851432d>] l2tp_eth_dev_xmit+0x2d/0x40 [l2tp_eth]
[<c14fa32f>] dev_hard_start_xmit+0x49f/0x7e0
[<c14f9ee1>] ? dev_hard_start_xmit+0x51/0x7e0
[<c1515819>] sch_direct_xmit+0xa9/0x250
[<c16157e1>] ? _raw_spin_lock+0x61/0x70
[<c14fa835>] dev_queue_xmit+0x1c5/0xbd0
[<c14fa670>] ? dev_hard_start_xmit+0x7e0/0x7e0
[<c159442c>] ip6_finish_output2+0x11c/0x620
[<c159434f>] ? ip6_finish_output2+0x3f/0x620
[<c159813f>] ip6_finish_output+0x7f/0x1e0
[<c15982ea>] ip6_output+0x4a/0x1f0
[<c15a6ae0>] ? ip6_blackhole_route+0x2c0/0x2c0
[<c15bbddc>] mld_sendpack+0x21c/0x530
[<c15bbbc0>] ? igmp6_group_added+0x170/0x170
[<c15bc817>] mld_ifc_timer_expire+0x187/0x260
[<c1055d10>] run_timer_softirq+0x140/0x370
[<c1055c88>] ? run_timer_softirq+0xb8/0x370
[<c1085776>] ? rebalance_domains+0x1b6/0x2a0
[<c15bc690>] ? igmp6_timer_handler+0x80/0x80
[<c104da27>] __do_softirq+0x97/0x1f0
[<c104d990>] ? local_bh_enable_ip+0xd0/0xd0
<IRQ> [<c104ddce>] ? irq_exit+0x7e/0xa0
[<c161e0f9>] ? smp_apic_timer_interrupt+0x59/0x88
[<c12fb498>] ? trace_hardirqs_off_thunk+0xc/0x14
[<c1616882>] ? apic_timer_interrupt+0x36/0x3c
[<c10380d5>] ? native_safe_halt+0x5/0x10
[<c1018bdf>] ? default_idle+0x4f/0x1e0
[<c1018dc1>] ? amd_e400_idle+0x51/0x100
[<c10199c9>] ? cpu_idle+0xb9/0xe0
[<c16038fc>] ? start_secondary+0x1ea/0x1f0
And on AMD64 UP machine:
============================================
INFO: possible recursive locking detected ]
.5.0-rc2-net-lockdep-u64-sync-006-+ #2 Not tainted
--------------------------------------------
wapper/0/0 is trying to acquire lock:
(slock-AF_INET){+.-...}, at: [<f864fbff>] l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
ut task is already holding lock:
(slock-AF_INET){+.-...}, at: [<c154c177>] tcp_delack_timer+0x17/0x1e0
ther info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(slock-AF_INET);
lock(slock-AF_INET);
*** DEADLOCK ***
May be due to missing lock nesting notation
locks held by swapper/0/0:
#0: (&icsk->icsk_delack_timer){+.-...}, at: [<c1055c88>] run_timer_softirq+0xb8/0x370
#1: (slock-AF_INET){+.-...}, at: [<c154c177>] tcp_delack_timer+0x17/0x1e0
#2: (rcu_read_lock){.+.+..}, at: [<c1531bf0>] ip_queue_xmit+0x0/0x610
#3: (rcu_read_lock){.+.+..}, at: [<c1531456>] ip_finish_output+0x106/0x710
#4: (rcu_read_lock_bh){.+....}, at: [<c14fa670>] dev_queue_xmit+0x0/0xbd0
tack backtrace:
id: 0, comm: swapper/0 Not tainted 3.5.0-rc2-net-lockdep-u64-sync-006-+ #2
all Trace:
[<c10a7b32>] __lock_acquire+0xd52/0x17d0
[<c1017ba8>] ? sched_clock+0x8/0x10
[<c107edbb>] ? sched_clock_local+0xcb/0x1c0
[<c10a8b48>] lock_acquire+0x88/0x120
[<f864fbff>] ? l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
[<c16157bb>] _raw_spin_lock+0x3b/0x70
[<f864fbff>] ? l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
[<f864fbff>] l2tp_xmit_skb+0x13f/0x8e0 [l2tp_core]
[<f853032d>] l2tp_eth_dev_xmit+0x2d/0x40 [l2tp_eth]
[<c14fa32f>] dev_hard_start_xmit+0x49f/0x7e0
[<c14f9ee1>] ? dev_hard_start_xmit+0x51/0x7e0
[<c1515819>] sch_direct_xmit+0xa9/0x250
[<c16157e1>] ? _raw_spin_lock+0x61/0x70
[<c14fa835>] dev_queue_xmit+0x1c5/0xbd0
[<c14fa670>] ? dev_hard_start_xmit+0x7e0/0x7e0
[<c15065f7>] neigh_resolve_output+0x117/0x230
[<c1514880>] ? eth_rebuild_header+0x80/0x80
[<c1531612>] ip_finish_output+0x2c2/0x710
[<c1531456>] ? ip_finish_output+0x106/0x710
[<c1532770>] ? ip_output+0x60/0x120
[<c10a585b>] ? trace_hardirqs_on+0xb/0x10
[<c153278b>] ip_output+0x7b/0x120
[<c1531b95>] ip_local_out+0x25/0x80
[<c1531d73>] ip_queue_xmit+0x183/0x610
[<c1531bf0>] ? ip_local_out+0x80/0x80
[<c154ecb5>] ? tcp_md5_do_lookup+0x125/0x170
[<c15498c6>] tcp_transmit_skb+0x396/0x970
[<c154bb12>] ? tcp_send_ack+0x32/0x100
[<c154bb9d>] tcp_send_ack+0xbd/0x100
[<c154c271>] tcp_delack_timer+0x111/0x1e0
[<c1055d10>] run_timer_softirq+0x140/0x370
[<c1055c88>] ? run_timer_softirq+0xb8/0x370
[<c154c160>] ? tcp_out_of_resources+0xb0/0xb0
[<c14f88cc>] ? net_rx_action+0x10c/0x210
[<c104da27>] __do_softirq+0x97/0x1f0
[<c104d990>] ? local_bh_enable_ip+0xd0/0xd0
<IRQ> [<c104ddce>] ? irq_exit+0x7e/0xa0
[<c161e02b>] ? do_IRQ+0x4b/0xc0
[<c161de75>] ? common_interrupt+0x35/0x3c
[<c10380d5>] ? native_safe_halt+0x5/0x10
[<c1018bdf>] ? default_idle+0x4f/0x1e0
[<c1018dc1>] ? amd_e400_idle+0x51/0x100
[<c10199c9>] ? cpu_idle+0xb9/0xe0
[<c15eab3e>] ? rest_init+0x112/0x124
[<c15eaa2c>] ? __read_lock_failed+0x14/0x14
[<c1907a11>] ? start_kernel+0x376/0x37c
[<c19074d6>] ? repair_env_string+0x51/0x51
[<c19072f8>] ? i386_start_kernel+0x9b/0xa2
--
Tom Parkin
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH] can: flexcan: use be32_to_cpup to handle the value of dt entry
From: Shawn Guo @ 2012-06-27 11:26 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: davem, netdev, linux-can, Hui Wang
In-Reply-To: <1340789236-28266-2-git-send-email-mkl@pengutronix.de>
On 27 June 2012 17:27, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> From: Hui Wang <jason77.wang@gmail.com>
>
> The freescale arm i.MX series platform can support this driver, and
> usually the arm cpu works in the little endian mode by default, while
> device tree entry value is stored in big endian format, we should use
> be32_to_cpup() to handle them, after modification, it can work well
> both on the le cpu and be cpu.
>
I'm wondering if you want to just use of_property_read_u32() to make
it a little bit easier.
Regards,
Shawn
> Cc: stable <stable@vger.kernel.org> # v3.2+
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Signed-off-by: Hui Wang <jason77.wang@gmail.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> drivers/net/can/flexcan.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 38c0690..81d4741 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -939,12 +939,12 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> return PTR_ERR(pinctrl);
>
> if (pdev->dev.of_node) {
> - const u32 *clock_freq_p;
> + const __be32 *clock_freq_p;
>
> clock_freq_p = of_get_property(pdev->dev.of_node,
> "clock-frequency", NULL);
> if (clock_freq_p)
> - clock_freq = *clock_freq_p;
> + clock_freq = be32_to_cpup(clock_freq_p);
> }
>
> if (!clock_freq) {
> --
> 1.7.10
>
^ permalink raw reply
* Re: [PATCH] can: flexcan: use be32_to_cpup to handle the value of dt entry
From: Marc Kleine-Budde @ 2012-06-27 11:34 UTC (permalink / raw)
To: Shawn Guo; +Cc: davem, netdev, linux-can, Hui Wang
In-Reply-To: <CAAQ0ZWT3ovG7eKZZuoSZkx4FGd+W9XvnTk8R1yuD02u4wqGNHQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 927 bytes --]
On 06/27/2012 01:26 PM, Shawn Guo wrote:
> On 27 June 2012 17:27, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>> From: Hui Wang <jason77.wang@gmail.com>
>>
>> The freescale arm i.MX series platform can support this driver, and
>> usually the arm cpu works in the little endian mode by default, while
>> device tree entry value is stored in big endian format, we should use
>> be32_to_cpup() to handle them, after modification, it can work well
>> both on the le cpu and be cpu.
>>
> I'm wondering if you want to just use of_property_read_u32() to make
> it a little bit easier.
Even better. Hui can you send a updated patch.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* [net-next patch] bnx2x: Define bnx2x_tests_str_arr as const
From: Merav Sicron @ 2012-06-27 11:52 UTC (permalink / raw)
To: davem, netdev, eilong; +Cc: Merav Sicron
This patch changes the definition of bnx2x_tests_str_arr from static char to
static const char.
Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Merav Sicron <meravs@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
Hi Dave,
Please consider applying this patch to net-next.
Thanks,
Merav
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index bff3129..81cadb6 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -1600,7 +1600,7 @@ static int bnx2x_set_pauseparam(struct net_device *dev,
return 0;
}
-static char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
+static const char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
"register_test (offline) ",
"memory_test (offline) ",
"int_loopback_test (offline)",
--
1.7.10
^ permalink raw reply related
* RE: [net-next patch] bnx2x: Define bnx2x_tests_str_arr as const
From: David Laight @ 2012-06-27 11:59 UTC (permalink / raw)
To: Merav Sicron, davem, netdev, eilong
In-Reply-To: <1340797976-13827-1-git-send-email-meravs@broadcom.com>
> -static char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
> +static const char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
> "register_test (offline) ",
> "memory_test (offline) ",
> "int_loopback_test (offline)",
You are still missing a 'const'.
You probably want:
static const char *const bnx2x_tests_str_arr[] ...
However if you are going to pad the strings to [28]
you might as well remove the layer of indirection - ie:
static const char bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF][28] = { ...
}
Or pad to 32 chars to (probably) remove some code bytes.
David
^ permalink raw reply
* [PATCH v2] l2tp: use per-cpu variables for u64_stats updates
From: Tom Parkin @ 2012-06-27 12:00 UTC (permalink / raw)
To: netdev; +Cc: David.Laight, Tom Parkin, James Chapman
This patch fixes a race condition in l2tp when updating tunnel and
session statistics. Previously it was possible for multiple threads
to concurrently call u64_stats_update*(), which lead to statistics
readers blocking forever.
This race was discovered on an AMD64 SMP machine running a 32bit
kernel. Running "ip l2tp" while sending data over an Ethernet
pseudowire resulted in an occasional soft lockup in
u64_stats_fetch_begin() called from l2tp_nl_session_send().
For safe lockless update of l2tp stats, data is now stored in per-cpu
variables. These per-cpu datasets are then summed at read time via.
an extra helper function l2tp_stats_copy() which has been added to
l2tp_core.c.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
net/l2tp/l2tp_core.c | 286 ++++++++++++++++++++++++++++-------------------
net/l2tp/l2tp_core.h | 44 ++++++--
net/l2tp/l2tp_debugfs.c | 42 ++++---
net/l2tp/l2tp_netlink.c | 64 ++++-------
net/l2tp/l2tp_ppp.c | 75 ++++++++-----
5 files changed, 301 insertions(+), 210 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 32b2155..ab2ffc0 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -320,6 +320,43 @@ struct l2tp_tunnel *l2tp_tunnel_find_nth(struct net *net, int nth)
}
EXPORT_SYMBOL_GPL(l2tp_tunnel_find_nth);
+/*
+ * Sum tunnel/session statistics across all CPUs
+ */
+int l2tp_stats_copy(struct l2tp_stats *cpustats, struct l2tp_stats *dest)
+{
+ int i;
+ unsigned int start;
+
+ if (!cpustats || !dest)
+ return 1;
+
+ memset(dest, 0, sizeof(struct l2tp_stats));
+
+ for_each_possible_cpu(i) {
+ struct l2tp_stats *stats = per_cpu_ptr(cpustats, i);
+
+ do {
+ start = u64_stats_fetch_begin(&stats->tx.syncp);
+ dest->tx.packets += stats->tx.packets;
+ dest->tx.bytes += stats->tx.bytes;
+ dest->tx.errors += stats->tx.errors;
+ } while (u64_stats_fetch_retry(&stats->tx.syncp, start));
+
+ do {
+ start = u64_stats_fetch_begin(&stats->rx.syncp);
+ dest->rx.packets += stats->rx.packets;
+ dest->rx.bytes += stats->rx.bytes;
+ dest->rx.errors += stats->rx.errors;
+ dest->rx.seq_discards += stats->rx.seq_discards;
+ dest->rx.oos_packets += stats->rx.oos_packets;
+ } while (u64_stats_fetch_retry(&stats->rx.syncp, start));
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(l2tp_stats_copy);
+
/*****************************************************************************
* Receive data handling
*****************************************************************************/
@@ -335,7 +372,6 @@ static void l2tp_recv_queue_skb(struct l2tp_session *session, struct sk_buff *sk
struct l2tp_stats *sstats;
spin_lock_bh(&session->reorder_q.lock);
- sstats = &session->stats;
skb_queue_walk_safe(&session->reorder_q, skbp, tmp) {
if (L2TP_SKB_CB(skbp)->ns > ns) {
__skb_queue_before(&session->reorder_q, skbp, skb);
@@ -343,9 +379,10 @@ static void l2tp_recv_queue_skb(struct l2tp_session *session, struct sk_buff *sk
"%s: pkt %hu, inserted before %hu, reorder_q len=%d\n",
session->name, ns, L2TP_SKB_CB(skbp)->ns,
skb_queue_len(&session->reorder_q));
- u64_stats_update_begin(&sstats->syncp);
- sstats->rx_oos_packets++;
- u64_stats_update_end(&sstats->syncp);
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.oos_packets++;
+ u64_stats_update_end(&sstats->rx.syncp);
goto out;
}
}
@@ -369,16 +406,17 @@ static void l2tp_recv_dequeue_skb(struct l2tp_session *session, struct sk_buff *
*/
skb_orphan(skb);
- tstats = &tunnel->stats;
- u64_stats_update_begin(&tstats->syncp);
- sstats = &session->stats;
- u64_stats_update_begin(&sstats->syncp);
- tstats->rx_packets++;
- tstats->rx_bytes += length;
- sstats->rx_packets++;
- sstats->rx_bytes += length;
- u64_stats_update_end(&tstats->syncp);
- u64_stats_update_end(&sstats->syncp);
+ tstats = this_cpu_ptr(tunnel->cpustats);
+ u64_stats_update_begin(&tstats->rx.syncp);
+ tstats->rx.packets++;
+ tstats->rx.bytes += length;
+ u64_stats_update_end(&tstats->rx.syncp);
+
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.packets++;
+ sstats->rx.bytes += length;
+ u64_stats_update_end(&sstats->rx.syncp);
if (L2TP_SKB_CB(skb)->has_seq) {
/* Bump our Nr */
@@ -417,13 +455,13 @@ static void l2tp_recv_dequeue(struct l2tp_session *session)
*/
start:
spin_lock_bh(&session->reorder_q.lock);
- sstats = &session->stats;
skb_queue_walk_safe(&session->reorder_q, skb, tmp) {
if (time_after(jiffies, L2TP_SKB_CB(skb)->expires)) {
- u64_stats_update_begin(&sstats->syncp);
- sstats->rx_seq_discards++;
- sstats->rx_errors++;
- u64_stats_update_end(&sstats->syncp);
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.seq_discards++;
+ sstats->rx.errors++;
+ u64_stats_update_end(&sstats->rx.syncp);
l2tp_dbg(session, L2TP_MSG_SEQ,
"%s: oos pkt %u len %d discarded (too old), waiting for %u, reorder_q_len=%d\n",
session->name, L2TP_SKB_CB(skb)->ns,
@@ -582,7 +620,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
struct l2tp_tunnel *tunnel = session->tunnel;
int offset;
u32 ns, nr;
- struct l2tp_stats *sstats = &session->stats;
+ struct l2tp_stats *sstats;
/* The ref count is increased since we now hold a pointer to
* the session. Take care to decrement the refcnt when exiting
@@ -599,9 +637,10 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
"%s: cookie mismatch (%u/%u). Discarding.\n",
tunnel->name, tunnel->tunnel_id,
session->session_id);
- u64_stats_update_begin(&sstats->syncp);
- sstats->rx_cookie_discards++;
- u64_stats_update_end(&sstats->syncp);
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.cookie_discards++;
+ u64_stats_update_end(&sstats->rx.syncp);
goto discard;
}
ptr += session->peer_cookie_len;
@@ -670,9 +709,10 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
l2tp_warn(session, L2TP_MSG_SEQ,
"%s: recv data has no seq numbers when required. Discarding.\n",
session->name);
- u64_stats_update_begin(&sstats->syncp);
- sstats->rx_seq_discards++;
- u64_stats_update_end(&sstats->syncp);
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.seq_discards++;
+ u64_stats_update_end(&sstats->rx.syncp);
goto discard;
}
@@ -691,9 +731,10 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
l2tp_warn(session, L2TP_MSG_SEQ,
"%s: recv data has no seq numbers when required. Discarding.\n",
session->name);
- u64_stats_update_begin(&sstats->syncp);
- sstats->rx_seq_discards++;
- u64_stats_update_end(&sstats->syncp);
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.seq_discards++;
+ u64_stats_update_end(&sstats->rx.syncp);
goto discard;
}
}
@@ -747,9 +788,10 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
* packets
*/
if (L2TP_SKB_CB(skb)->ns != session->nr) {
- u64_stats_update_begin(&sstats->syncp);
- sstats->rx_seq_discards++;
- u64_stats_update_end(&sstats->syncp);
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.seq_discards++;
+ u64_stats_update_end(&sstats->rx.syncp);
l2tp_dbg(session, L2TP_MSG_SEQ,
"%s: oos pkt %u len %d discarded, waiting for %u, reorder_q_len=%d\n",
session->name, L2TP_SKB_CB(skb)->ns,
@@ -775,9 +817,10 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
return;
discard:
- u64_stats_update_begin(&sstats->syncp);
- sstats->rx_errors++;
- u64_stats_update_end(&sstats->syncp);
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.errors++;
+ u64_stats_update_end(&sstats->rx.syncp);
kfree_skb(skb);
if (session->deref)
@@ -891,10 +934,10 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb,
discard_bad_csum:
LIMIT_NETDEBUG("%s: UDP: bad checksum\n", tunnel->name);
UDP_INC_STATS_USER(tunnel->l2tp_net, UDP_MIB_INERRORS, 0);
- tstats = &tunnel->stats;
- u64_stats_update_begin(&tstats->syncp);
- tstats->rx_errors++;
- u64_stats_update_end(&tstats->syncp);
+ tstats = this_cpu_ptr(tunnel->cpustats);
+ u64_stats_update_begin(&tstats->rx.syncp);
+ tstats->rx.errors++;
+ u64_stats_update_end(&tstats->rx.syncp);
kfree_skb(skb);
return 0;
@@ -1050,21 +1093,21 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb,
error = ip_queue_xmit(skb, fl);
/* Update stats */
- tstats = &tunnel->stats;
- u64_stats_update_begin(&tstats->syncp);
- sstats = &session->stats;
- u64_stats_update_begin(&sstats->syncp);
+ tstats = this_cpu_ptr(tunnel->cpustats);
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&tstats->tx.syncp);
+ u64_stats_update_begin(&sstats->tx.syncp);
if (error >= 0) {
- tstats->tx_packets++;
- tstats->tx_bytes += len;
- sstats->tx_packets++;
- sstats->tx_bytes += len;
+ tstats->tx.packets++;
+ tstats->tx.bytes += len;
+ sstats->tx.packets++;
+ sstats->tx.bytes += len;
} else {
- tstats->tx_errors++;
- sstats->tx_errors++;
+ tstats->tx.errors++;
+ sstats->tx.errors++;
}
- u64_stats_update_end(&tstats->syncp);
- u64_stats_update_end(&sstats->syncp);
+ u64_stats_update_end(&tstats->tx.syncp);
+ u64_stats_update_end(&sstats->tx.syncp);
return 0;
}
@@ -1256,6 +1299,8 @@ static void l2tp_tunnel_destruct(struct sock *sk)
sk->sk_destruct = tunnel->old_sk_destruct;
sk->sk_user_data = NULL;
+ free_percpu(tunnel->cpustats);
+
/* Call the original destructor */
if (sk->sk_destruct)
(*sk->sk_destruct)(sk);
@@ -1567,6 +1612,13 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
goto err;
}
+ tunnel->cpustats = alloc_percpu(struct l2tp_stats);
+ if (tunnel->cpustats == NULL) {
+ kfree(tunnel);
+ err = -ENOMEM;
+ goto err;
+ }
+
tunnel->version = version;
tunnel->tunnel_id = tunnel_id;
tunnel->peer_tunnel_id = peer_tunnel_id;
@@ -1692,6 +1744,8 @@ void l2tp_session_free(struct l2tp_session *session)
sock_put(tunnel->sock);
+ free_percpu(session->cpustats);
+
/* This will delete the tunnel context if this
* is the last session on the tunnel.
*/
@@ -1742,80 +1796,88 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
struct l2tp_session *session;
session = kzalloc(sizeof(struct l2tp_session) + priv_size, GFP_KERNEL);
- if (session != NULL) {
- session->magic = L2TP_SESSION_MAGIC;
- session->tunnel = tunnel;
+ if (session == NULL)
+ goto err;
- session->session_id = session_id;
- session->peer_session_id = peer_session_id;
- session->nr = 0;
+ session->cpustats = alloc_percpu(struct l2tp_stats);
+ if (session->cpustats == NULL) {
+ kfree(session);
+ goto err;
+ }
- sprintf(&session->name[0], "sess %u/%u",
- tunnel->tunnel_id, session->session_id);
+ session->magic = L2TP_SESSION_MAGIC;
+ session->tunnel = tunnel;
- skb_queue_head_init(&session->reorder_q);
-
- INIT_HLIST_NODE(&session->hlist);
- INIT_HLIST_NODE(&session->global_hlist);
-
- /* Inherit debug options from tunnel */
- session->debug = tunnel->debug;
-
- if (cfg) {
- session->pwtype = cfg->pw_type;
- session->debug = cfg->debug;
- session->mtu = cfg->mtu;
- session->mru = cfg->mru;
- session->send_seq = cfg->send_seq;
- session->recv_seq = cfg->recv_seq;
- session->lns_mode = cfg->lns_mode;
- session->reorder_timeout = cfg->reorder_timeout;
- session->offset = cfg->offset;
- session->l2specific_type = cfg->l2specific_type;
- session->l2specific_len = cfg->l2specific_len;
- session->cookie_len = cfg->cookie_len;
- memcpy(&session->cookie[0], &cfg->cookie[0], cfg->cookie_len);
- session->peer_cookie_len = cfg->peer_cookie_len;
- memcpy(&session->peer_cookie[0], &cfg->peer_cookie[0], cfg->peer_cookie_len);
- }
+ session->session_id = session_id;
+ session->peer_session_id = peer_session_id;
+ session->nr = 0;
- if (tunnel->version == L2TP_HDR_VER_2)
- session->build_header = l2tp_build_l2tpv2_header;
- else
- session->build_header = l2tp_build_l2tpv3_header;
+ sprintf(&session->name[0], "sess %u/%u",
+ tunnel->tunnel_id, session->session_id);
- l2tp_session_set_header_len(session, tunnel->version);
+ skb_queue_head_init(&session->reorder_q);
+
+ INIT_HLIST_NODE(&session->hlist);
+ INIT_HLIST_NODE(&session->global_hlist);
+
+ /* Inherit debug options from tunnel */
+ session->debug = tunnel->debug;
+
+ if (cfg) {
+ session->pwtype = cfg->pw_type;
+ session->debug = cfg->debug;
+ session->mtu = cfg->mtu;
+ session->mru = cfg->mru;
+ session->send_seq = cfg->send_seq;
+ session->recv_seq = cfg->recv_seq;
+ session->lns_mode = cfg->lns_mode;
+ session->reorder_timeout = cfg->reorder_timeout;
+ session->offset = cfg->offset;
+ session->l2specific_type = cfg->l2specific_type;
+ session->l2specific_len = cfg->l2specific_len;
+ session->cookie_len = cfg->cookie_len;
+ memcpy(&session->cookie[0], &cfg->cookie[0], cfg->cookie_len);
+ session->peer_cookie_len = cfg->peer_cookie_len;
+ memcpy(&session->peer_cookie[0], &cfg->peer_cookie[0], cfg->peer_cookie_len);
+ }
- /* Bump the reference count. The session context is deleted
- * only when this drops to zero.
- */
- l2tp_session_inc_refcount(session);
- l2tp_tunnel_inc_refcount(tunnel);
+ if (tunnel->version == L2TP_HDR_VER_2)
+ session->build_header = l2tp_build_l2tpv2_header;
+ else
+ session->build_header = l2tp_build_l2tpv3_header;
- /* Ensure tunnel socket isn't deleted */
- sock_hold(tunnel->sock);
+ l2tp_session_set_header_len(session, tunnel->version);
- /* Add session to the tunnel's hash list */
- write_lock_bh(&tunnel->hlist_lock);
- hlist_add_head(&session->hlist,
- l2tp_session_id_hash(tunnel, session_id));
- write_unlock_bh(&tunnel->hlist_lock);
+ /* Bump the reference count. The session context is deleted
+ * only when this drops to zero.
+ */
+ l2tp_session_inc_refcount(session);
+ l2tp_tunnel_inc_refcount(tunnel);
- /* And to the global session list if L2TPv3 */
- if (tunnel->version != L2TP_HDR_VER_2) {
- struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net);
+ /* Ensure tunnel socket isn't deleted */
+ sock_hold(tunnel->sock);
- spin_lock_bh(&pn->l2tp_session_hlist_lock);
- hlist_add_head_rcu(&session->global_hlist,
- l2tp_session_id_hash_2(pn, session_id));
- spin_unlock_bh(&pn->l2tp_session_hlist_lock);
- }
+ /* Add session to the tunnel's hash list */
+ write_lock_bh(&tunnel->hlist_lock);
+ hlist_add_head(&session->hlist,
+ l2tp_session_id_hash(tunnel, session_id));
+ write_unlock_bh(&tunnel->hlist_lock);
- /* Ignore management session in session count value */
- if (session->session_id != 0)
- atomic_inc(&l2tp_session_count);
+ /* And to the global session list if L2TPv3 */
+ if (tunnel->version != L2TP_HDR_VER_2) {
+ struct l2tp_net *pn = l2tp_pernet(tunnel->l2tp_net);
+
+ spin_lock_bh(&pn->l2tp_session_hlist_lock);
+ hlist_add_head_rcu(&session->global_hlist,
+ l2tp_session_id_hash_2(pn, session_id));
+ spin_unlock_bh(&pn->l2tp_session_hlist_lock);
}
+ /* Ignore management session in session count value */
+ if (session->session_id != 0)
+ atomic_inc(&l2tp_session_count);
+
+err:
return session;
}
EXPORT_SYMBOL_GPL(l2tp_session_create);
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index a38ec6c..29bb34c 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -35,19 +35,34 @@ enum {
struct sk_buff;
-struct l2tp_stats {
- u64 tx_packets;
- u64 tx_bytes;
- u64 tx_errors;
- u64 rx_packets;
- u64 rx_bytes;
- u64 rx_seq_discards;
- u64 rx_oos_packets;
- u64 rx_errors;
- u64 rx_cookie_discards;
+/* Stats are synchronised via a synchronisation point for safe
+ * reader/writer access on 64 and 32 bit kernels. Stats are
+ * stored on a per-cpu basis for safe lockless updating, and
+ * summed at read time. This allows the statistics update in
+ * the data path to be fast at some small cost when reading.
+ */
+struct l2tp_rx_stats {
+ u64 packets;
+ u64 bytes;
+ u64 errors;
+ u64 seq_discards;
+ u64 cookie_discards;
+ u64 oos_packets;
+ struct u64_stats_sync syncp;
+};
+
+struct l2tp_tx_stats {
+ u64 packets;
+ u64 bytes;
+ u64 errors;
struct u64_stats_sync syncp;
};
+struct l2tp_stats {
+ struct l2tp_rx_stats rx;
+ struct l2tp_tx_stats tx;
+};
+
struct l2tp_tunnel;
/* Describes a session. Contains information to determine incoming
@@ -127,7 +142,9 @@ struct l2tp_session {
int mtu;
int mru;
enum l2tp_pwtype pwtype;
- struct l2tp_stats stats;
+
+ struct l2tp_stats *cpustats; /* per-cpu counters */
+
struct hlist_node global_hlist; /* Global hash list node */
int (*build_header)(struct l2tp_session *session, void *buf);
@@ -175,7 +192,8 @@ struct l2tp_tunnel {
int debug; /* bitmask of debug message
* categories */
enum l2tp_encap_type encap;
- struct l2tp_stats stats;
+
+ struct l2tp_stats *cpustats; /* per-cpu counters */
struct list_head list; /* Keep a list of all tunnels */
struct net *l2tp_net; /* the net we belong to */
@@ -246,6 +264,8 @@ extern int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int
extern int l2tp_nl_register_ops(enum l2tp_pwtype pw_type, const struct l2tp_nl_cmd_ops *ops);
extern void l2tp_nl_unregister_ops(enum l2tp_pwtype pw_type);
+extern int l2tp_stats_copy(struct l2tp_stats *cpustats, struct l2tp_stats *dest);
+
/* Session reference counts. Incremented when code obtains a reference
* to a session.
*/
diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
index c3813bc..5ec4732 100644
--- a/net/l2tp/l2tp_debugfs.c
+++ b/net/l2tp/l2tp_debugfs.c
@@ -106,6 +106,7 @@ static void l2tp_dfs_seq_tunnel_show(struct seq_file *m, void *v)
int hash;
struct hlist_node *walk;
struct hlist_node *tmp;
+ struct l2tp_stats stats;
read_lock_bh(&tunnel->hlist_lock);
for (hash = 0; hash < L2TP_HASH_SIZE; hash++) {
@@ -146,14 +147,18 @@ static void l2tp_dfs_seq_tunnel_show(struct seq_file *m, void *v)
tunnel->sock ? atomic_read(&tunnel->sock->sk_refcnt) : 0,
atomic_read(&tunnel->ref_count));
- seq_printf(m, " %08x rx %llu/%llu/%llu rx %llu/%llu/%llu\n",
- tunnel->debug,
- (unsigned long long)tunnel->stats.tx_packets,
- (unsigned long long)tunnel->stats.tx_bytes,
- (unsigned long long)tunnel->stats.tx_errors,
- (unsigned long long)tunnel->stats.rx_packets,
- (unsigned long long)tunnel->stats.rx_bytes,
- (unsigned long long)tunnel->stats.rx_errors);
+ if (!l2tp_stats_copy(tunnel->cpustats, &stats)) {
+ seq_printf(m, " %08x tx %llu/%llu/%llu rx %llu/%llu/%llu\n",
+ tunnel->debug,
+ (unsigned long long)stats.tx.packets,
+ (unsigned long long)stats.tx.bytes,
+ (unsigned long long)stats.tx.errors,
+ (unsigned long long)stats.rx.packets,
+ (unsigned long long)stats.rx.bytes,
+ (unsigned long long)stats.rx.errors);
+ } else {
+ seq_printf(m, " stats lookup failure\n");
+ }
if (tunnel->show != NULL)
tunnel->show(m, tunnel);
@@ -162,6 +167,7 @@ static void l2tp_dfs_seq_tunnel_show(struct seq_file *m, void *v)
static void l2tp_dfs_seq_session_show(struct seq_file *m, void *v)
{
struct l2tp_session *session = v;
+ struct l2tp_stats stats;
seq_printf(m, " SESSION %u, peer %u, %s\n", session->session_id,
session->peer_session_id,
@@ -203,14 +209,18 @@ static void l2tp_dfs_seq_session_show(struct seq_file *m, void *v)
seq_printf(m, "\n");
}
- seq_printf(m, " %hu/%hu tx %llu/%llu/%llu rx %llu/%llu/%llu\n",
- session->nr, session->ns,
- (unsigned long long)session->stats.tx_packets,
- (unsigned long long)session->stats.tx_bytes,
- (unsigned long long)session->stats.tx_errors,
- (unsigned long long)session->stats.rx_packets,
- (unsigned long long)session->stats.rx_bytes,
- (unsigned long long)session->stats.rx_errors);
+ if (!l2tp_stats_copy(session->cpustats, &stats)) {
+ seq_printf(m, " %hu/%hu tx %llu/%llu/%llu rx %llu/%llu/%llu\n",
+ session->nr, session->ns,
+ (unsigned long long)stats.tx.packets,
+ (unsigned long long)stats.tx.bytes,
+ (unsigned long long)stats.tx.errors,
+ (unsigned long long)stats.rx.packets,
+ (unsigned long long)stats.rx.bytes,
+ (unsigned long long)stats.rx.errors);
+ } else {
+ seq_printf(m, " stats lookup failure\n");
+ }
if (session->show != NULL)
session->show(m, session);
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index ddc553e..09fd33d 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -246,7 +246,6 @@ static int l2tp_nl_tunnel_send(struct sk_buff *skb, u32 pid, u32 seq, int flags,
struct ipv6_pinfo *np = NULL;
#endif
struct l2tp_stats stats;
- unsigned int start;
hdr = genlmsg_put(skb, pid, seq, &l2tp_nl_family, flags,
L2TP_CMD_TUNNEL_GET);
@@ -264,28 +263,19 @@ static int l2tp_nl_tunnel_send(struct sk_buff *skb, u32 pid, u32 seq, int flags,
if (nest == NULL)
goto nla_put_failure;
- do {
- start = u64_stats_fetch_begin(&tunnel->stats.syncp);
- stats.tx_packets = tunnel->stats.tx_packets;
- stats.tx_bytes = tunnel->stats.tx_bytes;
- stats.tx_errors = tunnel->stats.tx_errors;
- stats.rx_packets = tunnel->stats.rx_packets;
- stats.rx_bytes = tunnel->stats.rx_bytes;
- stats.rx_errors = tunnel->stats.rx_errors;
- stats.rx_seq_discards = tunnel->stats.rx_seq_discards;
- stats.rx_oos_packets = tunnel->stats.rx_oos_packets;
- } while (u64_stats_fetch_retry(&tunnel->stats.syncp, start));
-
- if (nla_put_u64(skb, L2TP_ATTR_TX_PACKETS, stats.tx_packets) ||
- nla_put_u64(skb, L2TP_ATTR_TX_BYTES, stats.tx_bytes) ||
- nla_put_u64(skb, L2TP_ATTR_TX_ERRORS, stats.tx_errors) ||
- nla_put_u64(skb, L2TP_ATTR_RX_PACKETS, stats.rx_packets) ||
- nla_put_u64(skb, L2TP_ATTR_RX_BYTES, stats.rx_bytes) ||
+ if (0 != l2tp_stats_copy(tunnel->cpustats, &stats))
+ goto nla_put_failure;
+
+ if (nla_put_u64(skb, L2TP_ATTR_TX_PACKETS, stats.tx.packets) ||
+ nla_put_u64(skb, L2TP_ATTR_TX_BYTES, stats.tx.bytes) ||
+ nla_put_u64(skb, L2TP_ATTR_TX_ERRORS, stats.tx.errors) ||
+ nla_put_u64(skb, L2TP_ATTR_RX_PACKETS, stats.rx.packets) ||
+ nla_put_u64(skb, L2TP_ATTR_RX_BYTES, stats.rx.bytes) ||
nla_put_u64(skb, L2TP_ATTR_RX_SEQ_DISCARDS,
- stats.rx_seq_discards) ||
+ stats.rx.seq_discards) ||
nla_put_u64(skb, L2TP_ATTR_RX_OOS_PACKETS,
- stats.rx_oos_packets) ||
- nla_put_u64(skb, L2TP_ATTR_RX_ERRORS, stats.rx_errors))
+ stats.rx.oos_packets) ||
+ nla_put_u64(skb, L2TP_ATTR_RX_ERRORS, stats.rx.errors))
goto nla_put_failure;
nla_nest_end(skb, nest);
@@ -612,7 +602,6 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 pid, u32 seq, int flags
struct l2tp_tunnel *tunnel = session->tunnel;
struct sock *sk = NULL;
struct l2tp_stats stats;
- unsigned int start;
sk = tunnel->sock;
@@ -655,28 +644,19 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 pid, u32 seq, int flags
if (nest == NULL)
goto nla_put_failure;
- do {
- start = u64_stats_fetch_begin(&session->stats.syncp);
- stats.tx_packets = session->stats.tx_packets;
- stats.tx_bytes = session->stats.tx_bytes;
- stats.tx_errors = session->stats.tx_errors;
- stats.rx_packets = session->stats.rx_packets;
- stats.rx_bytes = session->stats.rx_bytes;
- stats.rx_errors = session->stats.rx_errors;
- stats.rx_seq_discards = session->stats.rx_seq_discards;
- stats.rx_oos_packets = session->stats.rx_oos_packets;
- } while (u64_stats_fetch_retry(&session->stats.syncp, start));
-
- if (nla_put_u64(skb, L2TP_ATTR_TX_PACKETS, stats.tx_packets) ||
- nla_put_u64(skb, L2TP_ATTR_TX_BYTES, stats.tx_bytes) ||
- nla_put_u64(skb, L2TP_ATTR_TX_ERRORS, stats.tx_errors) ||
- nla_put_u64(skb, L2TP_ATTR_RX_PACKETS, stats.rx_packets) ||
- nla_put_u64(skb, L2TP_ATTR_RX_BYTES, stats.rx_bytes) ||
+ if (0 != l2tp_stats_copy(session->cpustats, &stats))
+ goto nla_put_failure;
+
+ if (nla_put_u64(skb, L2TP_ATTR_TX_PACKETS, stats.tx.packets) ||
+ nla_put_u64(skb, L2TP_ATTR_TX_BYTES, stats.tx.bytes) ||
+ nla_put_u64(skb, L2TP_ATTR_TX_ERRORS, stats.tx.errors) ||
+ nla_put_u64(skb, L2TP_ATTR_RX_PACKETS, stats.rx.packets) ||
+ nla_put_u64(skb, L2TP_ATTR_RX_BYTES, stats.rx.bytes) ||
nla_put_u64(skb, L2TP_ATTR_RX_SEQ_DISCARDS,
- stats.rx_seq_discards) ||
+ stats.rx.seq_discards) ||
nla_put_u64(skb, L2TP_ATTR_RX_OOS_PACKETS,
- stats.rx_oos_packets) ||
- nla_put_u64(skb, L2TP_ATTR_RX_ERRORS, stats.rx_errors))
+ stats.rx.oos_packets) ||
+ nla_put_u64(skb, L2TP_ATTR_RX_ERRORS, stats.rx.errors))
goto nla_put_failure;
nla_nest_end(skb, nest);
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 286366e..054c069 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -222,6 +222,7 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
{
struct pppol2tp_session *ps = l2tp_session_priv(session);
struct sock *sk = NULL;
+ struct l2tp_stats *sstats = NULL;
/* If the socket is bound, send it in to PPP's input queue. Otherwise
* queue it on the session socket.
@@ -259,7 +260,10 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
session->name);
/* Not bound. Nothing we can do, so discard. */
- session->stats.rx_errors++;
+ sstats = this_cpu_ptr(session->cpustats);
+ u64_stats_update_begin(&sstats->rx.syncp);
+ sstats->rx.errors++;
+ u64_stats_update_end(&sstats->rx.syncp);
kfree_skb(skb);
}
@@ -1028,16 +1032,21 @@ end:
****************************************************************************/
static void pppol2tp_copy_stats(struct pppol2tp_ioc_stats *dest,
- struct l2tp_stats *stats)
+ struct l2tp_stats *cpustats)
{
- dest->tx_packets = stats->tx_packets;
- dest->tx_bytes = stats->tx_bytes;
- dest->tx_errors = stats->tx_errors;
- dest->rx_packets = stats->rx_packets;
- dest->rx_bytes = stats->rx_bytes;
- dest->rx_seq_discards = stats->rx_seq_discards;
- dest->rx_oos_packets = stats->rx_oos_packets;
- dest->rx_errors = stats->rx_errors;
+ struct l2tp_stats tmp;
+
+ if (0 != l2tp_stats_copy(cpustats, &tmp))
+ return;
+
+ dest->tx_packets = tmp.tx.packets;
+ dest->tx_bytes = tmp.tx.bytes;
+ dest->tx_errors = tmp.tx.errors;
+ dest->rx_packets = tmp.rx.packets;
+ dest->rx_bytes = tmp.rx.bytes;
+ dest->rx_seq_discards = tmp.rx.seq_discards;
+ dest->rx_oos_packets = tmp.rx.oos_packets;
+ dest->rx_errors = tmp.rx.errors;
}
/* Session ioctl helper.
@@ -1151,7 +1160,7 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
memset(&stats, 0, sizeof(stats));
stats.tunnel_id = tunnel->tunnel_id;
stats.session_id = session->session_id;
- pppol2tp_copy_stats(&stats, &session->stats);
+ pppol2tp_copy_stats(&stats, session->cpustats);
if (copy_to_user((void __user *) arg, &stats,
sizeof(stats)))
break;
@@ -1214,7 +1223,7 @@ static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel,
#ifdef CONFIG_XFRM
stats.using_ipsec = (sk->sk_policy[0] || sk->sk_policy[1]) ? 1 : 0;
#endif
- pppol2tp_copy_stats(&stats, &tunnel->stats);
+ pppol2tp_copy_stats(&stats, tunnel->cpustats);
if (copy_to_user((void __user *) arg, &stats, sizeof(stats))) {
err = -EFAULT;
break;
@@ -1666,19 +1675,24 @@ static void pppol2tp_seq_stop(struct seq_file *p, void *v)
static void pppol2tp_seq_tunnel_show(struct seq_file *m, void *v)
{
struct l2tp_tunnel *tunnel = v;
+ struct l2tp_stats tmp;
seq_printf(m, "\nTUNNEL '%s', %c %d\n",
tunnel->name,
(tunnel == tunnel->sock->sk_user_data) ? 'Y' : 'N',
atomic_read(&tunnel->ref_count) - 1);
- seq_printf(m, " %08x %llu/%llu/%llu %llu/%llu/%llu\n",
- tunnel->debug,
- (unsigned long long)tunnel->stats.tx_packets,
- (unsigned long long)tunnel->stats.tx_bytes,
- (unsigned long long)tunnel->stats.tx_errors,
- (unsigned long long)tunnel->stats.rx_packets,
- (unsigned long long)tunnel->stats.rx_bytes,
- (unsigned long long)tunnel->stats.rx_errors);
+ if (!l2tp_stats_copy(tunnel->cpustats, &tmp)) {
+ seq_printf(m, " %08x %llu/%llu/%llu %llu/%llu/%llu\n",
+ tunnel->debug,
+ (unsigned long long)tmp.tx.packets,
+ (unsigned long long)tmp.tx.bytes,
+ (unsigned long long)tmp.tx.errors,
+ (unsigned long long)tmp.rx.packets,
+ (unsigned long long)tmp.rx.bytes,
+ (unsigned long long)tmp.rx.errors);
+ } else {
+ seq_printf(m, " stats lookup failure\n");
+ }
}
static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
@@ -1687,6 +1701,7 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
struct l2tp_tunnel *tunnel = session->tunnel;
struct pppol2tp_session *ps = l2tp_session_priv(session);
struct pppox_sock *po = pppox_sk(ps->sock);
+ struct l2tp_stats tmp;
u32 ip = 0;
u16 port = 0;
@@ -1713,14 +1728,18 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
session->lns_mode ? "LNS" : "LAC",
session->debug,
jiffies_to_msecs(session->reorder_timeout));
- seq_printf(m, " %hu/%hu %llu/%llu/%llu %llu/%llu/%llu\n",
- session->nr, session->ns,
- (unsigned long long)session->stats.tx_packets,
- (unsigned long long)session->stats.tx_bytes,
- (unsigned long long)session->stats.tx_errors,
- (unsigned long long)session->stats.rx_packets,
- (unsigned long long)session->stats.rx_bytes,
- (unsigned long long)session->stats.rx_errors);
+ if (!l2tp_stats_copy(session->cpustats, &tmp)) {
+ seq_printf(m, " %hu/%hu %llu/%llu/%llu %llu/%llu/%llu\n",
+ session->nr, session->ns,
+ (unsigned long long)tmp.tx.packets,
+ (unsigned long long)tmp.tx.bytes,
+ (unsigned long long)tmp.tx.errors,
+ (unsigned long long)tmp.rx.packets,
+ (unsigned long long)tmp.rx.bytes,
+ (unsigned long long)tmp.rx.errors);
+ } else {
+ seq_printf(m, " stats lookup failure\n");
+ }
if (po)
seq_printf(m, " interface %s\n", ppp_dev_name(&po->chan));
--
1.7.9.5
^ permalink raw reply related
* Re: BUG: No init found on NFSROOT
From: Fengguang Wu @ 2012-06-27 12:23 UTC (permalink / raw)
To: Christoph Lameter
Cc: Trond Myklebust, J. Bruce Fields, linux-nfs, LKML, netdev,
Pekka Enberg, Linux Memory Management List
In-Reply-To: <20120626172918.GA16446@localhost>
Hi Christoph,
It's a surprise that it bisects down to this commit. I confirmed
that it boots reliably if reverting this commit on top of linux-next.
8c138bc00925521c4e764269db3a903bd2a51592 is the first bad commit
commit 8c138bc00925521c4e764269db3a903bd2a51592
Author: Christoph Lameter <cl@linux.com>
Date: Wed Jun 13 10:24:58 2012 -0500
slab: Get rid of obj_size macro
The size of the slab object is frequently needed. Since we now
have a size field directly in the kmem_cache structure there is no
need anymore of the obj_size macro/function.
Signed-off-by: Christoph Lameter <cl@linux.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
:040000 040000 e0418be654b66b2364add59bb469024fd6958791 f6be0da4d4740844ab8a4c561dbe3815a3f9b8b4 M mm
bisect run success
> > [ 133.909702] =========================
> > [ 133.910694] [ BUG: held lock freed! ]
> > [ 133.911700] 3.5.0-rc4+ #5 Not tainted
> > [ 133.912672] -------------------------
> > [ 133.912969] swapper/0/0 is freeing memory ffff88001233ce08-ffff88001233de07, with a lock still held there!
> > [ 133.912969] (slock-AF_INET-RPC/1){+.-...}, at: [<ffffffff82ae84ee>] tcp_v4_rcv+0x28b/0x6fc
> > [ 133.912969] 3 locks held by swapper/0/0:
> > [ 133.912969] #0: (rcu_read_lock){.+.+..}, at: [<ffffffff82a1ea8a>] rcu_lock_acquire+0x0/0x29
> > [ 133.912969] #1: (rcu_read_lock){.+.+..}, at: [<ffffffff82aca483>] rcu_lock_acquire.constprop.14+0x0/0x30
> > [ 133.912969] #2: (slock-AF_INET-RPC/1){+.-...}, at: [<ffffffff82ae84ee>] tcp_v4_rcv+0x28b/0x6fc
> > [ 133.912969]
> > [ 133.912969] stack backtrace:
> > [ 133.912969] Pid: 0, comm: swapper/0 Not tainted 3.5.0-rc4+ #5
> > [ 133.912969] Call Trace:
> > [ 133.912969] <IRQ> [<ffffffff810e09ae>] debug_check_no_locks_freed+0x109/0x14b
> > [ 133.912969] [<ffffffff811774e0>] kmem_cache_free+0x2e/0xa7
> > [ 133.912969] [<ffffffff82a191e5>] __kfree_skb+0x7f/0x83
> > [ 133.912969] [<ffffffff82adeccd>] tcp_ack+0x45d/0xc6a
> > [ 133.912969] [<ffffffff810c22ae>] ? local_clock+0x3b/0x52
> > [ 133.912969] [<ffffffff82adff44>] tcp_rcv_state_process+0x15a/0x7c6
> > [ 133.912969] [<ffffffff82ae79e7>] tcp_v4_do_rcv+0x341/0x390
> > [ 133.912969] [<ffffffff82ae88db>] tcp_v4_rcv+0x678/0x6fc
> > [ 133.912969] [<ffffffff82aca618>] ip_local_deliver_finish+0x165/0x1e4
> > [ 133.912969] [<ffffffff82acab4a>] ip_local_deliver+0x53/0x84
> > [ 133.912969] [<ffffffff810c228c>] ? local_clock+0x19/0x52
> > [ 133.912969] [<ffffffff82aca9c6>] ip_rcv_finish+0x32f/0x367
> > [ 133.912969] [<ffffffff82acad8b>] ip_rcv+0x210/0x269
> > [ 133.912969] [<ffffffff82a1eab1>] ? rcu_lock_acquire+0x27/0x29
> > [ 133.912969] [<ffffffff82a1ea8a>] ? softnet_seq_show+0x68/0x68
> > [ 133.912969] [<ffffffff82a21ede>] __netif_receive_skb+0x3cd/0x464
> > [ 133.912969] [<ffffffff82a21fda>] netif_receive_skb+0x65/0x9c
> > [ 133.912969] [<ffffffff82a227c5>] ? __napi_gro_receive+0xf2/0xff
> > [ 133.912969] [<ffffffff82a2209e>] napi_skb_finish+0x26/0x58
> > [ 133.912969] [<ffffffff810c228c>] ? local_clock+0x19/0x52
> > [ 133.912969] [<ffffffff82a228c5>] napi_gro_receive+0x2f/0x34
> > [ 133.912969] [<ffffffff81e36d12>] e1000_receive_skb+0x57/0x60
> > [ 133.912969] [<ffffffff81e39b23>] e1000_clean_rx_irq+0x2f2/0x387
> > [ 133.912969] [<ffffffff81e390f3>] e1000_clean+0x541/0x695
> > [ 133.912969] [<ffffffff8106c57b>] ? kvm_clock_read+0x2e/0x36
> > [ 133.912969] [<ffffffff82a22402>] ? net_rx_action+0x1b3/0x1f8
> > [ 133.912969] [<ffffffff82a22302>] net_rx_action+0xb3/0x1f8
> > [ 133.912969] [<ffffffff810984ab>] ? __do_softirq+0x76/0x1e8
> > [ 133.912969] [<ffffffff81098515>] __do_softirq+0xe0/0x1e8
> > [ 133.912969] [<ffffffff81122190>] ? time_hardirqs_off+0x26/0x2a
> > [ 133.912969] [<ffffffff82ea7fec>] call_softirq+0x1c/0x30
> > [ 133.912969] [<ffffffff81049cc8>] do_softirq+0x4a/0xa2
> > [ 133.912969] [<ffffffff8109888e>] irq_exit+0x51/0xbc
> > [ 133.912969] [<ffffffff82ea88ae>] do_IRQ+0x8e/0xa5
> > [ 133.912969] [<ffffffff82ea002f>] common_interrupt+0x6f/0x6f
> > [ 133.912969] <EOI> [<ffffffff8106c76b>] ? native_safe_halt+0x6/0x8
> > [ 133.912969] [<ffffffff810e08a3>] ? trace_hardirqs_on+0xd/0xf
> > [ 133.912969] [<ffffffff8104f384>] default_idle+0x53/0x90
> > [ 133.912969] [<ffffffff8104fc09>] cpu_idle+0xcc/0x123
> > [ 133.912969] [<ffffffff82d1d8dd>] rest_init+0xd1/0xda
> > [ 133.912969] [<ffffffff82d1d80c>] ? csum_partial_copy_generic+0x16c/0x16c
> > [ 133.912969] [<ffffffff8460dbbc>] start_kernel+0x3da/0x3e7
> > [ 133.912969] [<ffffffff8460d5ea>] ? repair_env_string+0x5a/0x5a
> > [ 133.912969] [<ffffffff8460d2d6>] x86_64_start_reservations+0xb1/0xb5
> > [ 133.912969] [<ffffffff8460d3d8>] x86_64_start_kernel+0xfe/0x10b
> > [ 134.024230] VFS: Mounted root (nfs filesystem) on device 0:14.
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* RE: [net-next patch] bnx2x: Define bnx2x_tests_str_arr as const
From: Merav Sicron @ 2012-06-27 12:45 UTC (permalink / raw)
To: David Laight; +Cc: davem, netdev, eilong
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B6F68@saturn3.aculab.com>
On Wed, 2012-06-27 at 12:59 +0100, David Laight wrote:
> > -static char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
> > +static const char *bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF] = {
> > "register_test (offline) ",
> > "memory_test (offline) ",
> > "int_loopback_test (offline)",
>
> You are still missing a 'const'.
> You probably want:
> static const char *const bnx2x_tests_str_arr[] ...
>
> However if you are going to pad the strings to [28]
> you might as well remove the layer of indirection - ie:
> static const char bnx2x_tests_str_arr[BNX2X_NUM_TESTS_SF][28] = { ...
> }
> Or pad to 32 chars to (probably) remove some code bytes.
>
> David
>
Thanks David, let me think about it.
Dave, please ignore this patch for now.
Thanks,
Merav
^ permalink raw reply
* Re: [PATCH] sctp: be mroe restrictive in transport selection on bundled sacks
From: Vlad Yasevich @ 2012-06-27 13:20 UTC (permalink / raw)
To: Neil Horman; +Cc: David Miller, netdev, linux-sctp
In-Reply-To: <20120627102419.GA19313@hmsreliant.think-freely.org>
On 06/27/2012 06:24 AM, Neil Horman wrote:
> On Tue, Jun 26, 2012 at 09:05:04PM -0700, David Miller wrote:
>> From: Neil Horman<nhorman@tuxdriver.com>
>> Date: Tue, 26 Jun 2012 16:31:44 -0400
>>
>>> @@ -240,15 +240,20 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,
>>> */
>>> if (sctp_chunk_is_data(chunk)&& !pkt->has_sack&&
>>> !pkt->has_cookie_echo) {
>>> - struct sctp_association *asoc;
>>> struct timer_list *timer;
>>> - asoc = pkt->transport->asoc;
>>> + struct sctp_association *asoc = pkt->transport->asoc;
>>> + struct sctp_transport *trans;
>>> +
>>> timer =&asoc->timers[SCTP_EVENT_TIMEOUT_SACK];
>>>
>>> /* If the SACK timer is running, we have a pending SACK */
>>> if (timer_pending(timer)) {
>>> struct sctp_chunk *sack;
>>> asoc->a_rwnd = asoc->rwnd;
>>> +
>>> + if (chunk->transport&& !chunk->transport->moved_ctsn)
>>> + return retval;
>>> +
>>> sack = sctp_make_sack(asoc);
>>> if (sack) {
>>> retval = sctp_packet_append_chunk(pkt, sack);
>>
>> The new local variable 'trans' seems to be unused.
>>
> Crap, thank you Dave, that was a holdover from an initial pass I had made in
> writing this. I'll repost with that removed once Vald has a chance to look this
> over
> Neil
>
Also, may want to move a_rwnd adjustment to after the new if clause. No
sense adjusting it if we are just going to bail.
-vlad
^ permalink raw reply
* Re: [PATCH] sctp: be mroe restrictive in transport selection on bundled sacks
From: Neil Horman @ 2012-06-27 13:22 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: David Miller, netdev, linux-sctp
In-Reply-To: <4FEB08B9.7030907@gmail.com>
On Wed, Jun 27, 2012 at 09:20:57AM -0400, Vlad Yasevich wrote:
> On 06/27/2012 06:24 AM, Neil Horman wrote:
> >On Tue, Jun 26, 2012 at 09:05:04PM -0700, David Miller wrote:
> >>From: Neil Horman<nhorman@tuxdriver.com>
> >>Date: Tue, 26 Jun 2012 16:31:44 -0400
> >>
> >>>@@ -240,15 +240,20 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,
> >>> */
> >>> if (sctp_chunk_is_data(chunk)&& !pkt->has_sack&&
> >>> !pkt->has_cookie_echo) {
> >>>- struct sctp_association *asoc;
> >>> struct timer_list *timer;
> >>>- asoc = pkt->transport->asoc;
> >>>+ struct sctp_association *asoc = pkt->transport->asoc;
> >>>+ struct sctp_transport *trans;
> >>>+
> >>> timer =&asoc->timers[SCTP_EVENT_TIMEOUT_SACK];
> >>>
> >>> /* If the SACK timer is running, we have a pending SACK */
> >>> if (timer_pending(timer)) {
> >>> struct sctp_chunk *sack;
> >>> asoc->a_rwnd = asoc->rwnd;
> >>>+
> >>>+ if (chunk->transport&& !chunk->transport->moved_ctsn)
> >>>+ return retval;
> >>>+
> >>> sack = sctp_make_sack(asoc);
> >>> if (sack) {
> >>> retval = sctp_packet_append_chunk(pkt, sack);
> >>
> >>The new local variable 'trans' seems to be unused.
> >>
> >Crap, thank you Dave, that was a holdover from an initial pass I had made in
> >writing this. I'll repost with that removed once Vald has a chance to look this
> >over
> >Neil
> >
>
> Also, may want to move a_rwnd adjustment to after the new if clause.
> No sense adjusting it if we are just going to bail.
>
> -vlad
>
copy that, I'll make both changes and repost. Thanks!
Neil
^ permalink raw reply
* Re: [RFC] tcp demux used to signal ip_route_input_noref to not cache dst
From: Hans Schillstrom @ 2012-06-27 13:25 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: netdev
In-Reply-To: <1340783533.26242.2.camel@edumazet-glaptop>
On Wednesday 27 June 2012 09:52:13 Eric Dumazet wrote:
> On Wed, 2012-06-27 at 09:19 +0200, Eric Dumazet wrote:
> > In case tcp_v{4|6}_early_demux() doesnt find an ESTABLISHED socket, and
> > SYN flag is set, and an "atomic_t listener_under_synflood" counter is
> > not 0, we could :
> >
> > - instruct make ip_rcv_finish() to not cache the input dst into route
> > cache (if dst is not found in the hash table)
> >
> > This would make synflood attacks having minimal impact on route cache
> >
> > (We did this for the output dst of SYN-cookie-ACK messages)
> >
> >
>
> I'll test the following patch in a moment.
>
> For the moment, set nocache to true for all frames not associated to an
> ESTABLISHED socket. Not sure we want to test SYN flag after all.
Nice work,
I have been runing the patch for almost 4 hours now
not a single message about the routing cache !
BTW
I also use the "tcp: avoid tx starvation by SYNACK packets" patch
and jhash patch for syn cookies.
Not a packet single packet is dropped now.
I even works nice in a KVM I have never been close to this results with KVM.
^ permalink raw reply
* [PATCH v2 0/5] fec driver updates
From: Shawn Guo @ 2012-06-27 13:45 UTC (permalink / raw)
To: David S. Miller
Cc: Lothar Waßmann, Florian Fainelli, netdev, linux-arm-kernel,
devicetree-discuss, Shawn Guo
Changes since v1:
* Add one patch to use devm_gpio_request_one
* Have a separate patch to fix phy-reset-gpios property in binding
document
* Change phy-reset-interval to phy-reset-duration
* Add a sanity check on phy-reset-duration value
Shawn Guo (5):
net: fec: reset phy after pinctrl setup
net: fec: enable regulator for fec phy
net: fec: use managed function devm_gpio_request_one
net: fec: phy-reset-gpios is optional
net: fec: add phy-reset-duration for device tree probe
Documentation/devicetree/bindings/net/fsl-fec.txt | 6 +++-
drivers/net/ethernet/freescale/fec.c | 28 ++++++++++++++++++---
2 files changed, 29 insertions(+), 5 deletions(-)
--
1.7.5.4
^ permalink raw reply
* [PATCH v2 1/5] net: fec: reset phy after pinctrl setup
From: Shawn Guo @ 2012-06-27 13:45 UTC (permalink / raw)
To: David S. Miller
Cc: Lothar Waßmann, Florian Fainelli, netdev, linux-arm-kernel,
devicetree-discuss, Shawn Guo
In-Reply-To: <1340804724-29410-1-git-send-email-shawn.guo@linaro.org>
In case that bootloader or platform initialization does not set up
fec pins, the fec_reset_phy will not be able to succeed, because
fec_reset_phy is currently called before devm_pinctrl_get_select_default.
Move fec_reset_phy call to the place between devm_pinctrl_get_select_default
and fec_enet_init to have above case be taken care.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/net/ethernet/freescale/fec.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index ff7f4c5..e868a37 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1593,8 +1593,6 @@ fec_probe(struct platform_device *pdev)
fep->phy_interface = ret;
}
- fec_reset_phy(pdev);
-
for (i = 0; i < FEC_IRQ_NUM; i++) {
irq = platform_get_irq(pdev, i);
if (irq < 0) {
@@ -1634,6 +1632,8 @@ fec_probe(struct platform_device *pdev)
clk_prepare_enable(fep->clk_ahb);
clk_prepare_enable(fep->clk_ipg);
+ fec_reset_phy(pdev);
+
ret = fec_enet_init(ndev);
if (ret)
goto failed_init;
--
1.7.5.4
^ permalink raw reply related
* [PATCH v2 2/5] net: fec: enable regulator for fec phy
From: Shawn Guo @ 2012-06-27 13:45 UTC (permalink / raw)
To: David S. Miller
Cc: Lothar Waßmann, Florian Fainelli, netdev, linux-arm-kernel,
devicetree-discuss, Shawn Guo
In-Reply-To: <1340804724-29410-1-git-send-email-shawn.guo@linaro.org>
If bootloader or platform initialization code does not enable the
power supply to fec phy, we need to do it in fec driver before calling
fec_reset_phy to have the phy powered on.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/net/ethernet/freescale/fec.c | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index e868a37..4dce9e3 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -49,6 +49,7 @@
#include <linux/of_gpio.h>
#include <linux/of_net.h>
#include <linux/pinctrl/consumer.h>
+#include <linux/regulator/consumer.h>
#include <asm/cacheflush.h>
@@ -1546,6 +1547,7 @@ fec_probe(struct platform_device *pdev)
const struct of_device_id *of_id;
static int dev_id;
struct pinctrl *pinctrl;
+ struct regulator *reg_phy;
of_id = of_match_device(fec_dt_ids, &pdev->dev);
if (of_id)
@@ -1632,6 +1634,16 @@ fec_probe(struct platform_device *pdev)
clk_prepare_enable(fep->clk_ahb);
clk_prepare_enable(fep->clk_ipg);
+ reg_phy = devm_regulator_get(&pdev->dev, "phy");
+ if (!IS_ERR(reg_phy)) {
+ ret = regulator_enable(reg_phy);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Failed to enable phy regulator: %d\n", ret);
+ goto failed_regulator;
+ }
+ }
+
fec_reset_phy(pdev);
ret = fec_enet_init(ndev);
@@ -1655,6 +1667,7 @@ failed_register:
fec_enet_mii_remove(fep);
failed_mii_init:
failed_init:
+failed_regulator:
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
failed_pin:
--
1.7.5.4
^ permalink raw reply related
* [PATCH v2 3/5] net: fec: use managed function devm_gpio_request_one
From: Shawn Guo @ 2012-06-27 13:45 UTC (permalink / raw)
To: David S. Miller
Cc: Lothar Waßmann, Florian Fainelli, netdev, linux-arm-kernel,
devicetree-discuss, Shawn Guo
In-Reply-To: <1340804724-29410-1-git-send-email-shawn.guo@linaro.org>
Using gpio_request_one will require the probe fail-out call gpio_free,
which is missing currently. Change to use devm_gpio_request_one to
fix the problem.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
drivers/net/ethernet/freescale/fec.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 4dce9e3..f174070 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1513,7 +1513,8 @@ static void __devinit fec_reset_phy(struct platform_device *pdev)
return;
phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
- err = gpio_request_one(phy_reset, GPIOF_OUT_INIT_LOW, "phy-reset");
+ err = devm_gpio_request_one(&pdev->dev, phy_reset,
+ GPIOF_OUT_INIT_LOW, "phy-reset");
if (err) {
pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
return;
--
1.7.5.4
^ permalink raw reply related
* [PATCH v2 4/5] net: fec: phy-reset-gpios is optional
From: Shawn Guo @ 2012-06-27 13:45 UTC (permalink / raw)
To: David S. Miller
Cc: Lothar Waßmann, Florian Fainelli, netdev, linux-arm-kernel,
devicetree-discuss, Shawn Guo
In-Reply-To: <1340804724-29410-1-git-send-email-shawn.guo@linaro.org>
The phy-reset-gpios is an optional property for fec device tree boot.
Change the binding document to match the driver code.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Documentation/devicetree/bindings/net/fsl-fec.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 7ab9e1a..0428920 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -7,10 +7,10 @@ Required properties:
- phy-mode : String, operation mode of the PHY interface.
Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
"rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
-- phy-reset-gpios : Should specify the gpio for phy reset
Optional properties:
- local-mac-address : 6 bytes, mac address
+- phy-reset-gpios : Should specify the gpio for phy reset
Example:
--
1.7.5.4
^ permalink raw reply related
* [PATCH v2 5/5] net: fec: add phy-reset-duration for device tree probe
From: Shawn Guo @ 2012-06-27 13:45 UTC (permalink / raw)
To: David S. Miller
Cc: Lothar Waßmann, Florian Fainelli, netdev, linux-arm-kernel,
devicetree-discuss, Shawn Guo
In-Reply-To: <1340804724-29410-1-git-send-email-shawn.guo@linaro.org>
Different boards may require different phy reset duration. Add property
phy-reset-duration for device tree probe, so that the boards that need
a longer reset duration can specify it in their device tree.
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Documentation/devicetree/bindings/net/fsl-fec.txt | 4 ++++
drivers/net/ethernet/freescale/fec.c | 8 +++++++-
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 0428920..f7a2fef 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -11,6 +11,10 @@ Required properties:
Optional properties:
- local-mac-address : 6 bytes, mac address
- phy-reset-gpios : Should specify the gpio for phy reset
+- phy-reset-duration : Reset duration in milliseconds. Should present
+ only if property "phy-reset-gpios" is available. Missing the property
+ will have the duration be 1 millisecond. Numbers greater than 1000 are
+ invalid and 1 millisecond will be used instead.
Example:
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index f174070..dafd797 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1507,11 +1507,17 @@ static int __devinit fec_get_phy_mode_dt(struct platform_device *pdev)
static void __devinit fec_reset_phy(struct platform_device *pdev)
{
int err, phy_reset;
+ int msec = 1;
struct device_node *np = pdev->dev.of_node;
if (!np)
return;
+ of_property_read_u32(np, "phy-reset-duration", &msec);
+ /* A sane reset duration should not be longer than 1s */
+ if (msec > 1000)
+ msec = 1;
+
phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
err = devm_gpio_request_one(&pdev->dev, phy_reset,
GPIOF_OUT_INIT_LOW, "phy-reset");
@@ -1519,7 +1525,7 @@ static void __devinit fec_reset_phy(struct platform_device *pdev)
pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
return;
}
- msleep(1);
+ msleep(msec);
gpio_set_value(phy_reset, 1);
}
#else /* CONFIG_OF */
--
1.7.5.4
^ permalink raw reply related
* [PATCH v2] sctp: be more restrictive in transport selection on bundled sacks
From: Neil Horman @ 2012-06-27 14:23 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, Vlad Yaseivch, David S. Miller
In-Reply-To: <1340742704-2192-1-git-send-email-nhorman@tuxdriver.com>
It was noticed recently that when we send data on a transport, its possible that
we might bundle a sack that arrived on a different transport. While this isn't
a major problem, it does go against the SHOULD requirement in section 6.4 of RFC
2960:
An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK,
etc.) to the same destination transport address from which it
received the DATA or control chunk to which it is replying. This
rule should also be followed if the endpoint is bundling DATA chunks
together with the reply chunk.
This patch seeks to correct that. It restricts the bundling of sack operations
to only those transports which have moved the ctsn of the association forward
since the last sack. By doing this we guarantee that we only bundle outbound
saks on a transport that has received a chunk since the last sack. This brings
us into stricter compliance with the RFC.
Vlad had initially suggested that we strictly allow only sack bundling on the
transport that last moved the ctsn forward. While this makes sense, I was
concerned that doing so prevented us from bundling in the case where we had
received chunks that moved the ctsn on multiple transports. In those cases, the
RFC allows us to select any of the transports having received chunks to bundle
the sack on. so I've modified the approach to allow for that, by adding a state
variable to each transport that tracks weather it has moved the ctsn since the
last sack. This I think keeps our behavior (and performance), close enough to
our current profile that I think we can do this without a sysctl knob to
enable/disable it.
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Vlad Yaseivch <vyasevich@gmail.com>
CC: David S. Miller <davem@davemloft.net>
Reported-by: Michele Baldessari <michele@redhat.com>
Reported-by: sorin serban <sserban@redhat.com>
---
Change Notes:
V2)
* Removed unused variable as per Dave M. Request
* Delayed rwnd adjustment until we are sure we will sack (Vlad Y.)
---
include/net/sctp/structs.h | 5 ++++-
include/net/sctp/tsnmap.h | 3 ++-
net/sctp/output.c | 10 +++++++---
net/sctp/sm_make_chunk.c | 7 +++++++
net/sctp/sm_sideeffect.c | 2 +-
net/sctp/tsnmap.c | 5 ++++-
net/sctp/ulpevent.c | 3 ++-
net/sctp/ulpqueue.c | 2 +-
8 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index e4652fe..712bf09 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -910,7 +910,10 @@ struct sctp_transport {
pmtu_pending:1,
/* Is this structure kfree()able? */
- malloced:1;
+ malloced:1,
+
+ /* Has this transport moved the ctsn since we last sacked */
+ moved_ctsn:1;
struct flowi fl;
diff --git a/include/net/sctp/tsnmap.h b/include/net/sctp/tsnmap.h
index e7728bc..2c5d2b4 100644
--- a/include/net/sctp/tsnmap.h
+++ b/include/net/sctp/tsnmap.h
@@ -117,7 +117,8 @@ void sctp_tsnmap_free(struct sctp_tsnmap *map);
int sctp_tsnmap_check(const struct sctp_tsnmap *, __u32 tsn);
/* Mark this TSN as seen. */
-int sctp_tsnmap_mark(struct sctp_tsnmap *, __u32 tsn);
+int sctp_tsnmap_mark(struct sctp_tsnmap *, __u32 tsn,
+ struct sctp_transport *trans);
/* Mark this TSN and all lower as seen. */
void sctp_tsnmap_skip(struct sctp_tsnmap *map, __u32 tsn);
diff --git a/net/sctp/output.c b/net/sctp/output.c
index f1b7d4b..6c8cb9e 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -240,17 +240,21 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,
*/
if (sctp_chunk_is_data(chunk) && !pkt->has_sack &&
!pkt->has_cookie_echo) {
- struct sctp_association *asoc;
struct timer_list *timer;
- asoc = pkt->transport->asoc;
+ struct sctp_association *asoc = pkt->transport->asoc;
+
timer = &asoc->timers[SCTP_EVENT_TIMEOUT_SACK];
/* If the SACK timer is running, we have a pending SACK */
if (timer_pending(timer)) {
struct sctp_chunk *sack;
- asoc->a_rwnd = asoc->rwnd;
+
+ if (chunk->transport && !chunk->transport->moved_ctsn)
+ return retval;
+
sack = sctp_make_sack(asoc);
if (sack) {
+ asoc->a_rwnd = asoc->rwnd;
retval = sctp_packet_append_chunk(pkt, sack);
asoc->peer.sack_needed = 0;
if (del_timer(timer))
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index a85eeeb..805babe 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -736,6 +736,7 @@ struct sctp_chunk *sctp_make_sack(const struct sctp_association *asoc)
__u16 num_gabs, num_dup_tsns;
struct sctp_tsnmap *map = (struct sctp_tsnmap *)&asoc->peer.tsn_map;
struct sctp_gap_ack_block gabs[SCTP_MAX_GABS];
+ struct sctp_transport *trans;
memset(gabs, 0, sizeof(gabs));
ctsn = sctp_tsnmap_get_ctsn(map);
@@ -805,6 +806,12 @@ struct sctp_chunk *sctp_make_sack(const struct sctp_association *asoc)
sctp_addto_chunk(retval, sizeof(__u32) * num_dup_tsns,
sctp_tsnmap_get_dups(map));
+ /*
+ * Once we have a sack generated, clear the moved_tsn information
+ * from all the transports
+ */
+ list_for_each_entry(trans, &asoc->peer.transport_addr_list, transports)
+ trans->moved_ctsn = 0;
nodata:
return retval;
}
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index c96d1a8..8716da1 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1268,7 +1268,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
case SCTP_CMD_REPORT_TSN:
/* Record the arrival of a TSN. */
error = sctp_tsnmap_mark(&asoc->peer.tsn_map,
- cmd->obj.u32);
+ cmd->obj.u32, NULL);
break;
case SCTP_CMD_REPORT_FWDTSN:
diff --git a/net/sctp/tsnmap.c b/net/sctp/tsnmap.c
index f1e40ceb..619c638 100644
--- a/net/sctp/tsnmap.c
+++ b/net/sctp/tsnmap.c
@@ -114,7 +114,8 @@ int sctp_tsnmap_check(const struct sctp_tsnmap *map, __u32 tsn)
/* Mark this TSN as seen. */
-int sctp_tsnmap_mark(struct sctp_tsnmap *map, __u32 tsn)
+int sctp_tsnmap_mark(struct sctp_tsnmap *map, __u32 tsn,
+ struct sctp_transport *trans)
{
u16 gap;
@@ -133,6 +134,8 @@ int sctp_tsnmap_mark(struct sctp_tsnmap *map, __u32 tsn)
*/
map->max_tsn_seen++;
map->cumulative_tsn_ack_point++;
+ if (trans)
+ trans->moved_ctsn = 1;
map->base_tsn++;
} else {
/* Either we already have a gap, or about to record a gap, so
diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
index 8a84017..33d8947 100644
--- a/net/sctp/ulpevent.c
+++ b/net/sctp/ulpevent.c
@@ -715,7 +715,8 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
* can mark it as received so the tsn_map is updated correctly.
*/
if (sctp_tsnmap_mark(&asoc->peer.tsn_map,
- ntohl(chunk->subh.data_hdr->tsn)))
+ ntohl(chunk->subh.data_hdr->tsn),
+ chunk->transport))
goto fail_mark;
/* First calculate the padding, so we don't inadvertently
diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index f2d1de7..f5a6a4f 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -1051,7 +1051,7 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
if (chunk && (freed >= needed)) {
__u32 tsn;
tsn = ntohl(chunk->subh.data_hdr->tsn);
- sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn);
+ sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
sctp_ulpq_tail_data(ulpq, chunk, gfp);
sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH v2] sctp: be more restrictive in transport selection on bundled sacks
From: Vlad Yasevich @ 2012-06-27 15:10 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, David S. Miller
In-Reply-To: <1340807003-23139-1-git-send-email-nhorman@tuxdriver.com>
On 06/27/2012 10:23 AM, Neil Horman wrote:
> It was noticed recently that when we send data on a transport, its possible that
> we might bundle a sack that arrived on a different transport. While this isn't
> a major problem, it does go against the SHOULD requirement in section 6.4 of RFC
> 2960:
>
> An endpoint SHOULD transmit reply chunks (e.g., SACK, HEARTBEAT ACK,
> etc.) to the same destination transport address from which it
> received the DATA or control chunk to which it is replying. This
> rule should also be followed if the endpoint is bundling DATA chunks
> together with the reply chunk.
>
> This patch seeks to correct that. It restricts the bundling of sack operations
> to only those transports which have moved the ctsn of the association forward
> since the last sack. By doing this we guarantee that we only bundle outbound
> saks on a transport that has received a chunk since the last sack. This brings
> us into stricter compliance with the RFC.
>
> Vlad had initially suggested that we strictly allow only sack bundling on the
> transport that last moved the ctsn forward. While this makes sense, I was
> concerned that doing so prevented us from bundling in the case where we had
> received chunks that moved the ctsn on multiple transports. In those cases, the
> RFC allows us to select any of the transports having received chunks to bundle
> the sack on. so I've modified the approach to allow for that, by adding a state
> variable to each transport that tracks weather it has moved the ctsn since the
> last sack. This I think keeps our behavior (and performance), close enough to
> our current profile that I think we can do this without a sysctl knob to
> enable/disable it.
>
> Signed-off-by: Neil Horman<nhorman@tuxdriver.com>
> CC: Vlad Yaseivch<vyasevich@gmail.com>
> CC: David S. Miller<davem@davemloft.net>
> Reported-by: Michele Baldessari<michele@redhat.com>
> Reported-by: sorin serban<sserban@redhat.com>
>
> ---
> Change Notes:
> V2)
> * Removed unused variable as per Dave M. Request
> * Delayed rwnd adjustment until we are sure we will sack (Vlad Y.)
> ---
> include/net/sctp/structs.h | 5 ++++-
> include/net/sctp/tsnmap.h | 3 ++-
> net/sctp/output.c | 10 +++++++---
> net/sctp/sm_make_chunk.c | 7 +++++++
> net/sctp/sm_sideeffect.c | 2 +-
> net/sctp/tsnmap.c | 5 ++++-
> net/sctp/ulpevent.c | 3 ++-
> net/sctp/ulpqueue.c | 2 +-
> 8 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index e4652fe..712bf09 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -910,7 +910,10 @@ struct sctp_transport {
> pmtu_pending:1,
>
> /* Is this structure kfree()able? */
> - malloced:1;
> + malloced:1,
> +
> + /* Has this transport moved the ctsn since we last sacked */
> + moved_ctsn:1;
>
> struct flowi fl;
>
> diff --git a/include/net/sctp/tsnmap.h b/include/net/sctp/tsnmap.h
> index e7728bc..2c5d2b4 100644
> --- a/include/net/sctp/tsnmap.h
> +++ b/include/net/sctp/tsnmap.h
> @@ -117,7 +117,8 @@ void sctp_tsnmap_free(struct sctp_tsnmap *map);
> int sctp_tsnmap_check(const struct sctp_tsnmap *, __u32 tsn);
>
> /* Mark this TSN as seen. */
> -int sctp_tsnmap_mark(struct sctp_tsnmap *, __u32 tsn);
> +int sctp_tsnmap_mark(struct sctp_tsnmap *, __u32 tsn,
> + struct sctp_transport *trans);
>
> /* Mark this TSN and all lower as seen. */
> void sctp_tsnmap_skip(struct sctp_tsnmap *map, __u32 tsn);
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index f1b7d4b..6c8cb9e 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -240,17 +240,21 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct sctp_packet *pkt,
> */
> if (sctp_chunk_is_data(chunk)&& !pkt->has_sack&&
> !pkt->has_cookie_echo) {
> - struct sctp_association *asoc;
> struct timer_list *timer;
> - asoc = pkt->transport->asoc;
> + struct sctp_association *asoc = pkt->transport->asoc;
> +
> timer =&asoc->timers[SCTP_EVENT_TIMEOUT_SACK];
>
> /* If the SACK timer is running, we have a pending SACK */
> if (timer_pending(timer)) {
> struct sctp_chunk *sack;
> - asoc->a_rwnd = asoc->rwnd;
> +
> + if (chunk->transport&& !chunk->transport->moved_ctsn)
> + return retval;
> +
I didn't think of this yesterday, but I think it would be much better to
use pkt->transport here since you are adding the chunk to the packet and
it will go out on the transport of the packet. You are also guaranteed
that pkt->transport is set.
> sack = sctp_make_sack(asoc);
> if (sack) {
> + asoc->a_rwnd = asoc->rwnd;
> retval = sctp_packet_append_chunk(pkt, sack);
> asoc->peer.sack_needed = 0;
> if (del_timer(timer))
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index a85eeeb..805babe 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -736,6 +736,7 @@ struct sctp_chunk *sctp_make_sack(const struct sctp_association *asoc)
> __u16 num_gabs, num_dup_tsns;
> struct sctp_tsnmap *map = (struct sctp_tsnmap *)&asoc->peer.tsn_map;
> struct sctp_gap_ack_block gabs[SCTP_MAX_GABS];
> + struct sctp_transport *trans;
>
> memset(gabs, 0, sizeof(gabs));
> ctsn = sctp_tsnmap_get_ctsn(map);
> @@ -805,6 +806,12 @@ struct sctp_chunk *sctp_make_sack(const struct sctp_association *asoc)
> sctp_addto_chunk(retval, sizeof(__u32) * num_dup_tsns,
> sctp_tsnmap_get_dups(map));
>
> + /*
> + * Once we have a sack generated, clear the moved_tsn information
> + * from all the transports
> + */
> + list_for_each_entry(trans,&asoc->peer.transport_addr_list, transports)
> + trans->moved_ctsn = 0;
> nodata:
> return retval;
> }
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index c96d1a8..8716da1 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1268,7 +1268,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
> case SCTP_CMD_REPORT_TSN:
> /* Record the arrival of a TSN. */
> error = sctp_tsnmap_mark(&asoc->peer.tsn_map,
> - cmd->obj.u32);
> + cmd->obj.u32, NULL);
> break;
>
> case SCTP_CMD_REPORT_FWDTSN:
> diff --git a/net/sctp/tsnmap.c b/net/sctp/tsnmap.c
> index f1e40ceb..619c638 100644
> --- a/net/sctp/tsnmap.c
> +++ b/net/sctp/tsnmap.c
> @@ -114,7 +114,8 @@ int sctp_tsnmap_check(const struct sctp_tsnmap *map, __u32 tsn)
>
>
> /* Mark this TSN as seen. */
> -int sctp_tsnmap_mark(struct sctp_tsnmap *map, __u32 tsn)
> +int sctp_tsnmap_mark(struct sctp_tsnmap *map, __u32 tsn,
> + struct sctp_transport *trans)
> {
> u16 gap;
>
> @@ -133,6 +134,8 @@ int sctp_tsnmap_mark(struct sctp_tsnmap *map, __u32 tsn)
> */
> map->max_tsn_seen++;
> map->cumulative_tsn_ack_point++;
> + if (trans)
> + trans->moved_ctsn = 1;
> map->base_tsn++;
> } else {
> /* Either we already have a gap, or about to record a gap, so
> diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
> index 8a84017..33d8947 100644
> --- a/net/sctp/ulpevent.c
> +++ b/net/sctp/ulpevent.c
> @@ -715,7 +715,8 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
> * can mark it as received so the tsn_map is updated correctly.
> */
> if (sctp_tsnmap_mark(&asoc->peer.tsn_map,
> - ntohl(chunk->subh.data_hdr->tsn)))
> + ntohl(chunk->subh.data_hdr->tsn),
> + chunk->transport))
> goto fail_mark;
>
> /* First calculate the padding, so we don't inadvertently
> diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
> index f2d1de7..f5a6a4f 100644
> --- a/net/sctp/ulpqueue.c
> +++ b/net/sctp/ulpqueue.c
> @@ -1051,7 +1051,7 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> if (chunk&& (freed>= needed)) {
> __u32 tsn;
> tsn = ntohl(chunk->subh.data_hdr->tsn);
> - sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn);
> + sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
> sctp_ulpq_tail_data(ulpq, chunk, gfp);
>
> sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
Also, I think you need to reset this bit in sctp_transport_reset().
Consider a potential association restart after SACKs have been received.
-vlad
^ permalink raw reply
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