* Re: [PATCH 3/3] net: TCP thin dupack
From: apetlund @ 2009-10-30 14:11 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Andreas Petlund, Netdev, LKML, shemminger, David Miller
> On Thu, 29 Oct 2009, apetlund@simula.no wrote:
>
>> I apologise that some of you received this mail more than once. My
email
>> client played a HTML-trick on me.
>> >> + /* If a thin stream is detected, retransmit after first
>> >> + * received dupack */
>> >> + if ((tp->thin_dupack || sysctl_tcp_force_thin_dupack) &&
>> >> + tcp_dupack_heurestics(tp) > 1 && tcp_stream_is_thin(tp))
+ return 1;
>> >> +
>> >> return 0;
>> >> }
>> >
>> > Have you tested it? ...I doubt this will work like you say and
>> retransmit
>> > something when the window is small. ...Besides, you should have built
>> this
>> > patch on top of the function rename you submitted earlier as after
>> DaveM
>> applied that this will no longer even compile...
>> >
>> > --
>> > i.
>> >
>> We have performed extensive tests mapping the effect of the patch you
commented on some months ago. Since then, the only change was the one
you
>> requested of switching tcp_fackets_out() with tcp_dupack_heurestics().
After inspecting the code, I believed the effect should be equal to the
previous, only making considerations for SACK and FACK availability.
Please tell if this will break the intended effect, and I will modify
the
>> patch accordingly.
>
> Ah, you're of course right. FACK retransmits the head always but RFC3517
mode doesn't. I think you'd need to artificially lower (ie., to
calculate)
> the dupthresh (from tp->reordering) to be 1 for it to work as intented.
>
>> Graphs from our tests of the original patch can be found at the
location
>> linked to below. I have tested the new one for functionality, but have
not et performed tests on this scope as the changes were minor. I will,
of
>> course, fix the function rename in the next iteration. Sorry for that.
http://folk.uio.no/apetlund/lktmp/
>
> You curiousity, have you run this more aggressive form of early
retransmit
> against the one ID gives? ...I checked your results but if I understood
them correctly the IDish early retransmit wasn't among the variants
used.
We have not implemented EFR for Linux TCP. We have, however, performed
tests where we compare the Free BSD implementation on SCTP with SCTP using
our proposed exp. bo. and dupACK modifications. I know that this is not
directly comparable, and link to this as a digression:
http://folk.uio.no/apetlund/lktmp/SCTP_thin_compare.pdf
If you are interested in our set of SCTP experiments, it is summarised in
the paper linked to below:
http://simula.no/research/networks/publications/Simula.ND.311/simula_pdf_file
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH 1/3] net: TCP thin-stream detection
From: apetlund @ 2009-10-30 13:53 UTC (permalink / raw)
To: Arnd Hannemann
Cc: Andreas Petlund, William Allen Simpson, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, shemminger@vyatta.com,
ilpo.jarvinen@helsinki.fi, davem@davemloft.net
> Andreas Petlund schrieb:
>> Den 28. okt. 2009 kl. 04.09 skrev William Allen Simpson:
>>> Andreas Petlund wrote:
>>>> +/* Determines whether this is a thin stream (which may suffer from +
* increased latency). Used to trigger latency-reducing mechanisms. + */
>>>> +static inline unsigned int tcp_stream_is_thin(const struct
>>>> tcp_sock *tp)
>>>> +{
>>>> + return tp->packets_out < 4;
>>>> +}
>>>> +
>>> This bothers me a bit. Having just looked at your Linux presentation,
and not (yet) read your papers, it seems much of your justification was
>>> with 1 packet per RTT. Here, you seem to be concentrating on 4,
probably
>>> because many implementations quickly ramp up to 4.
>> The limit of 4 packets in flight is based on the fact that less than 4
packets in flight makes fast retransmissions impossible, thus limiting the
retransmit options to timeout-retransmissions. The criterion is
>
> There is Limited Transmit! So this is generally not true.
>> therefore as conservative as possible while still serving its purpose.
If further losses occur, the exponential backoff will increase latency
further. The concept of using this limit is also discussed in the
Internet draft for Early Retransmit by Allman et al.:
>> http://www.icir.org/mallman/papers/draft-ietf-tcpm-early-rexmt-01.txt
>
> This ID is covering exactly the cases which Limited Transmit does not
cover and works "automagically" without help of application. So why not
just implement this ID?
As Ilpo writes, the mechanism we propose is simpler than the ID, and
slightly more aggressive. The reason why we chose this is as follows: 1)
The ID and Limited Transmit tries to prevent retransmission timeouts by
retransmitting more aggressively, thus keeping the congestion window open
even though congestion may be the limiting factor. If their limiting
conditions change, they still have higher sending rates available. The
thin-stream applications are not limited by congestion control. There is
therefore no motivation to prevent retransmission timeouts in order to
keep the congestion window open because in the thin-stream scenario, a
larger window is not needed, but we retransmit early only to reduce
application-layer latencies. 2) Our suggested implementation is simpler.
3) I believe that the reason why the ID has not been implemented in Linux
is that the motivation did not justify the achieved result. We have
analysed a wide range of time-dependent applications and found that they
very often produce thin streams due to transmissions being triggered by
human interaction. This changes the motivational picture since a thin
stream is an indicator of time-dependency.
Regards,
Andreas
^ permalink raw reply
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Francis Moreau @ 2009-10-30 13:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <38b2ab8a0910300533l4bbc9d3cj8812fe38726c8278@mail.gmail.com>
On Fri, Oct 30, 2009 at 1:33 PM, Francis Moreau <francis.moro@gmail.com> wrote:
> On Fri, Oct 30, 2009 at 12:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> This oops occurring again and again with SUNRPC finally gave me the right pointer.
>>
>> David, we added two years ago memory accounting to UDP, and this changed
>> requirements about calling skb_free_datagram() in the right context.
>>
>> I wish we had an ASSERT_SOCK_LOCKED() debugging facility :(
>>
>> Francis, would you please test following patch ?
>
> I'm applying it and testing it during a couple of days and see if
> something wrong still happens.
Hmm, with the patch applied on a 2.6.32-rc5, my machine locks hard
when starting nfsd.
--
Francis
^ permalink raw reply
* [PATCH]NET:KS8695: add API for get rx interrupt bit
From: Figo.zhang @ 2009-10-30 13:05 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
1. Add API Add k8695_get_rx_enable_bit() for get Rx interrupt
enable/status bit.
2. add some comment or document about some functions and variables.
3. update driver version to "1.02"
4. add napi_enable() and napi_disable() in open/close file method.
Signed-off-by: Figo.zhang <figo1802@gmail.com>
---
drivers/net/arm/ks8695net.c | 69 ++++++++++++++++++++++++++++++++++--------
1 files changed, 56 insertions(+), 13 deletions(-)
diff --git a/drivers/net/arm/ks8695net.c b/drivers/net/arm/ks8695net.c
index ed0b0f3..b4f5994 100644
--- a/drivers/net/arm/ks8695net.c
+++ b/drivers/net/arm/ks8695net.c
@@ -41,8 +41,7 @@
#include "ks8695net.h"
#define MODULENAME "ks8695_ether"
-#define MODULEVERSION "1.01"
-
+#define MODULEVERSION "1.02"
/*
* Transmit and device reset timeout, default 5 seconds.
@@ -98,6 +97,9 @@ struct ks8695_skbuff {
#define MAX_RX_DESC 16
#define MAX_RX_DESC_MASK 0xf
+/*napi_weight have better more than rx DMA buffers*/
+#define NAPI_WEIGHT 64
+
#define MAX_RXBUF_SIZE 0x700
#define TX_RING_DMA_SIZE (sizeof(struct tx_ring_desc) * MAX_TX_DESC)
@@ -123,6 +125,7 @@ enum ks8695_dtype {
* @dev: The platform device object for this interface
* @dtype: The type of this device
* @io_regs: The ioremapped registers for this interface
+ * @napi : Add support NAPI for Rx
* @rx_irq_name: The textual name of the RX IRQ from the platform data
* @tx_irq_name: The textual name of the TX IRQ from the platform data
* @link_irq_name: The textual name of the link IRQ from the
@@ -146,6 +149,7 @@ enum ks8695_dtype {
* @rx_ring_dma: The DMA mapped equivalent of rx_ring
* @rx_buffers: The sk_buff mappings for the RX ring
* @next_rx_desc_read: The next RX descriptor to read from on IRQ
+ * @rx_lock: A lock to protect Rx irq function
* @msg_enable: The flags for which messages to emit
*/
struct ks8695_priv {
@@ -398,11 +402,30 @@ ks8695_tx_irq(int irq, void *dev_id)
}
/**
+ * ks8695_get_rx_enable_bit - Get rx interrupt enable/status bit
+ * @ksp: Private data for the KS8695 Ethernet
+ *
+ * For KS8695 document:
+ * Interrupt Enable Register (offset 0xE204)
+ * Bit29 : WAN MAC Receive Interrupt Enable
+ * Bit16 : LAN MAC Receive Interrupt Enable
+ * Interrupt Status Register (Offset 0xF208)
+ * Bit29: WAN MAC Receive Status
+ * Bit16: LAN MAC Receive Status
+ * So, this Rx interrrupt enable/status bit number is equal
+ * as Rx IRQ number.
+ */
+static inline u32 ks8695_get_rx_enable_bit(struct ks8695_priv *ksp)
+{
+ return ksp->rx_irq;
+}
+
+/**
* ks8695_rx_irq - Receive IRQ handler
* @irq: The IRQ which went off (ignored)
* @dev_id: The net_device for the interrupt
*
- * Use NAPI to receive packets.
+ * Inform NAPI that packet reception needs to be scheduled
*/
static irqreturn_t
@@ -412,7 +435,7 @@ ks8695_rx_irq(int irq, void *dev_id)
struct ks8695_priv *ksp = netdev_priv(ndev);
unsigned long status;
- unsigned long mask_bit = 1 << ksp->rx_irq;
+ unsigned long mask_bit = 1 << ks8695_get_rx_enable_bit(ksp);
spin_lock(&ksp->rx_lock);
@@ -434,9 +457,15 @@ ks8695_rx_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static int ks8695_rx(struct net_device *ndev, int budget)
+/**
+ * ks8695_rx - Receive packets called by NAPI poll method
+ * @ksp: Private data for the KS8695 Ethernet
+ * @budget: The max packets would be receive
+ */
+
+static int ks8695_rx(struct ks8695_priv *ksp, int budget)
{
- struct ks8695_priv *ksp = netdev_priv(ndev);
+ struct net_device *ndev = ksp->ndev;
struct sk_buff *skb;
int buff_n;
u32 flags;
@@ -526,20 +555,32 @@ rx_finished:
/* And refill the buffers */
ks8695_refill_rxbuffers(ksp);
+
+ /* Kick the RX DMA engine, in case it became
+ * suspended */
+ ks8695_writereg(ksp, KS8695_DRSC, 0);
}
return received;
}
+
+/**
+ * ks8695_poll - Receive packet by NAPI poll method
+ * @ksp: Private data for the KS8695 Ethernet
+ * @budget: The remaining number packets for network subsystem
+ *
+ * Invoked by the network core when it requests for new
+ * packets from the driver
+ */
static int ks8695_poll(struct napi_struct *napi, int budget)
{
struct ks8695_priv *ksp = container_of(napi, struct ks8695_priv, napi);
- struct net_device *dev = ksp->ndev;
- unsigned long mask_bit = 1 << ksp->rx_irq;
- unsigned long isr = readl(KS8695_IRQ_VA + KS8695_INTEN);
+ unsigned long work_done;
- unsigned long work_done ;
+ unsigned long isr = readl(KS8695_IRQ_VA + KS8695_INTEN);
+ unsigned long mask_bit = 1 << ks8695_get_rx_enable_bit(ksp);
- work_done = ks8695_rx(dev, budget);
+ work_done = ks8695_rx(ksp, budget);
if (work_done < budget) {
unsigned long flags;
@@ -1302,6 +1343,7 @@ ks8695_stop(struct net_device *ndev)
struct ks8695_priv *ksp = netdev_priv(ndev);
netif_stop_queue(ndev);
+ napi_disable(&ksp->napi);
netif_carrier_off(ndev);
ks8695_shutdown(ksp);
@@ -1335,7 +1377,8 @@ ks8695_open(struct net_device *ndev)
ks8695_shutdown(ksp);
return ret;
}
-
+
+ napi_enable(&ksp->napi);
netif_start_queue(ndev);
return 0;
@@ -1521,7 +1564,7 @@ ks8695_probe(struct platform_device *pdev)
SET_ETHTOOL_OPS(ndev, &ks8695_ethtool_ops);
ndev->watchdog_timeo = msecs_to_jiffies(watchdog);
- netif_napi_add(ndev, &ksp->napi, ks8695_poll, 64);
+ netif_napi_add(ndev, &ksp->napi, ks8695_poll, NAPI_WEIGHT);
/* Retrieve the default MAC addr from the chip. */
/* The bootloader should have left it in there for us. */
^ permalink raw reply related
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Francis Moreau @ 2009-10-30 12:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <4AEACD88.8080108@gmail.com>
On Fri, Oct 30, 2009 at 12:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> This oops occurring again and again with SUNRPC finally gave me the right pointer.
>
> David, we added two years ago memory accounting to UDP, and this changed
> requirements about calling skb_free_datagram() in the right context.
>
> I wish we had an ASSERT_SOCK_LOCKED() debugging facility :(
>
> Francis, would you please test following patch ?
I'm applying it and testing it during a couple of days and see if
something wrong still happens.
Since I have no specific test case, I'll report if this oops happen
again after a couple of days (probably on next Wednesday).
--
Francis
^ permalink raw reply
* Re: [PATCHv2] gro: Name the GRO result enumeration type
From: Ben Hutchings @ 2009-10-30 11:48 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <20091029.213223.94736944.davem@davemloft.net>
On Thu, 2009-10-29 at 21:32 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Thu, 29 Oct 2009 18:26:15 +0000
>
> > This clarifies which return and parameter types are GRO result codes
> > and not RX result codes.
> >
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> > This replaces the previous patch 1/4 and avoids introducing compiler
> > warnings. The original patches 2-4 will still apply on top of it.
>
> You can't do this Ben.
>
> Changing this patch makes the follow-on patches not apply cleanly.
[...]
I'm sorry about that - a git rebase succeeded without manual attention,
so I believed they did.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Eric Dumazet @ 2009-10-30 11:27 UTC (permalink / raw)
To: Francis Moreau
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <38b2ab8a0910300144i7a3c190fi9aa3d079c9cdb754@mail.gmail.com>
Francis Moreau a écrit :
> Hello Eric,
>
> It seems I still have a related bug, please have a look to the following oops.
>
> This happened on a 2.6.32-rc5 where your patch is included.
>
> [107304.558821] nfsd: last server has exited, flushing export cache
> [107304.558848] ------------[ cut here ]------------
> [107304.558858] WARNING: at net/ipv4/af_inet.c:153
> inet_sock_destruct+0x161/0x17c()
> [107304.558862] Hardware name: P5K-VM
> [107304.558865] Modules linked in: kvm_intel kvm jfs loop nfsd lockd
> nfs_acl auth_rpcgss exportfs sunrpc [last unloaded: microcode]
> [107304.558889] Pid: 8198, comm: nfsd Tainted: G M 2.6.32-rc5 #25
> [107304.558892] Call Trace:
> [107304.558899] [<ffffffff81429f19>] ? inet_sock_destruct+0x161/0x17c
> [107304.558907] [<ffffffff810487e9>] warn_slowpath_common+0x7c/0xa9
> [107304.558914] [<ffffffff8104882a>] warn_slowpath_null+0x14/0x16
> [107304.558920] [<ffffffff81429f19>] inet_sock_destruct+0x161/0x17c
> [107304.558927] [<ffffffff813c8741>] __sk_free+0x23/0xe7
> [107304.558933] [<ffffffff813c8881>] sk_free+0x1f/0x21
> [107304.558939] [<ffffffff813c894b>] sk_common_release+0xc8/0xcd
> [107304.558944] [<ffffffff81420b59>] udp_lib_close+0xe/0x10
> [107304.558951] [<ffffffff814299bf>] inet_release+0x55/0x5c
> [107304.558957] [<ffffffff813c5aa9>] sock_release+0x1f/0x71
> [107304.558962] [<ffffffff813c5b22>] sock_close+0x27/0x2b
> [107304.558968] [<ffffffff810eb60f>] __fput+0xfb/0x1c0
> [107304.558973] [<ffffffff810eb6f1>] fput+0x1d/0x1f
> [107304.558995] [<ffffffffa0013e23>] svc_sock_free+0x40/0x56 [sunrpc]
> [107304.559018] [<ffffffffa001f392>] svc_xprt_free+0x43/0x53 [sunrpc]
> [107304.559038] [<ffffffffa001f34f>] ? svc_xprt_free+0x0/0x53 [sunrpc]
> [107304.559048] [<ffffffff811d9275>] kref_put+0x43/0x4f
> [107304.559069] [<ffffffffa001e67a>] svc_close_xprt+0x55/0x5e [sunrpc]
> [107304.559088] [<ffffffffa001e6d3>] svc_close_all+0x50/0x69 [sunrpc]
> [107304.559107] [<ffffffffa0012a2b>] svc_destroy+0x9e/0x142 [sunrpc]
> [107304.559126] [<ffffffffa0012b88>] svc_exit_thread+0xb9/0xc2 [sunrpc]
> [107304.559138] [<ffffffffa008981b>] ? nfsd+0x0/0x13f [nfsd]
> [107304.559149] [<ffffffffa0089940>] nfsd+0x125/0x13f [nfsd]
> [107304.559157] [<ffffffff810685e3>] kthread+0x82/0x8a
> [107304.559164] [<ffffffff8100c13a>] child_rip+0xa/0x20
> [107304.559172] [<ffffffff8100baad>] ? restore_args+0x0/0x30
> [107304.559179] [<ffffffff81068561>] ? kthread+0x0/0x8a
> [107304.559185] [<ffffffff8100c130>] ? child_rip+0x0/0x20
> [107304.559191] ---[ end trace c107131f4762168c ]---
> [107304.927931] NFSD: Using /var/lib/nfs/v4recovery as the NFSv4 state
> recovery directory
> [107304.932765] NFSD: starting 90-second grace period
>
This oops occurring again and again with SUNRPC finally gave me the right pointer.
David, we added two years ago memory accounting to UDP, and this changed
requirements about calling skb_free_datagram() in the right context.
I wish we had an ASSERT_SOCK_LOCKED() debugging facility :(
Francis, would you please test following patch ?
Thanks
[PATCH] net: fix sk_forward_alloc corruption
On UDP sockets, we must call skb_free_datagram() with socket locked,
or risk sk_forward_alloc corruption. This requirement is not respected
in SUNRPC.
Add a convenient helper, skb_free_datagram_locked() and use it in SUNRPC
Reported-by: Francis Moreau <francis.moro@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/skbuff.h | 2 ++
net/core/datagram.c | 10 +++++++++-
net/ipv4/udp.c | 4 +---
net/ipv6/udp.c | 4 +---
net/sunrpc/svcsock.c | 10 +++++-----
net/sunrpc/xprtsock.c | 2 +-
6 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index df7b23a..266878f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1757,6 +1757,8 @@ extern int skb_copy_datagram_const_iovec(const struct sk_buff *from,
int to_offset,
int size);
extern void skb_free_datagram(struct sock *sk, struct sk_buff *skb);
+extern void skb_free_datagram_locked(struct sock *sk,
+ struct sk_buff *skb);
extern int skb_kill_datagram(struct sock *sk, struct sk_buff *skb,
unsigned int flags);
extern __wsum skb_checksum(const struct sk_buff *skb, int offset,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 1c6cf3a..4ade301 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -224,6 +224,15 @@ void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
consume_skb(skb);
sk_mem_reclaim_partial(sk);
}
+EXPORT_SYMBOL(skb_free_datagram);
+
+void skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb)
+{
+ lock_sock(sk);
+ skb_free_datagram(sk, skb);
+ release_sock(sk);
+}
+EXPORT_SYMBOL(skb_free_datagram_locked);
/**
* skb_kill_datagram - Free a datagram skbuff forcibly
@@ -752,5 +761,4 @@ unsigned int datagram_poll(struct file *file, struct socket *sock,
EXPORT_SYMBOL(datagram_poll);
EXPORT_SYMBOL(skb_copy_and_csum_datagram_iovec);
EXPORT_SYMBOL(skb_copy_datagram_iovec);
-EXPORT_SYMBOL(skb_free_datagram);
EXPORT_SYMBOL(skb_recv_datagram);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index d0d436d..0fa9f70 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -999,9 +999,7 @@ try_again:
err = ulen;
out_free:
- lock_sock(sk);
- skb_free_datagram(sk, skb);
- release_sock(sk);
+ skb_free_datagram_locked(sk, skb);
out:
return err;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 3a60f12..cf538ed 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -288,9 +288,7 @@ try_again:
err = ulen;
out_free:
- lock_sock(sk);
- skb_free_datagram(sk, skb);
- release_sock(sk);
+ skb_free_datagram_locked(sk, skb);
out:
return err;
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index ccc5e83..1c246a4 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -111,7 +111,7 @@ static void svc_release_skb(struct svc_rqst *rqstp)
rqstp->rq_xprt_ctxt = NULL;
dprintk("svc: service %p, releasing skb %p\n", rqstp, skb);
- skb_free_datagram(svsk->sk_sk, skb);
+ skb_free_datagram_locked(svsk->sk_sk, skb);
}
}
@@ -578,7 +578,7 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp)
"svc: received unknown control message %d/%d; "
"dropping RPC reply datagram\n",
cmh->cmsg_level, cmh->cmsg_type);
- skb_free_datagram(svsk->sk_sk, skb);
+ skb_free_datagram_locked(svsk->sk_sk, skb);
return 0;
}
@@ -588,18 +588,18 @@ 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(svsk->sk_sk, skb);
+ skb_free_datagram_locked(svsk->sk_sk, skb);
return 0;
}
local_bh_enable();
- skb_free_datagram(svsk->sk_sk, skb);
+ skb_free_datagram_locked(svsk->sk_sk, skb);
} else {
/* we can use it in-place */
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(svsk->sk_sk, skb);
+ skb_free_datagram_locked(svsk->sk_sk, skb);
return 0;
}
rqstp->rq_xprt_ctxt = skb;
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 37c5475..d61be4a 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -883,7 +883,7 @@ static void xs_udp_data_ready(struct sock *sk, int len)
out_unlock:
spin_unlock(&xprt->transport_lock);
dropit:
- skb_free_datagram(sk, skb);
+ skb_free_datagram_locked(sk, skb);
out:
read_unlock(&sk->sk_callback_lock);
}
^ permalink raw reply related
* Re: [PATCH 2/8] param: use ops in struct kernel_param, rather than get and set fns directly
From: Rusty Russell @ 2009-10-30 11:13 UTC (permalink / raw)
To: Takashi Iwai
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
Ville Syrjala, Dmitry Torokhov, Alessandro Rubini,
Michal Januszewski, Trond Myklebust, J. Bruce Fields, Neil Brown,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <s5hvdhxyyl7.wl%tiwai-l3A5Bk7waGM@public.gmane.org>
On Fri, 30 Oct 2009 08:48:12 pm Takashi Iwai wrote:
> At Fri, 23 Oct 2009 00:51:28 +1030,
> Rusty Russell wrote:
> >
> > This is more kernel-ish, saves some space, and also allows us to
> > expand the ops without breaking all the callers who are happy for the
> > new members to be NULL.
> >
> > The few places which defined their own param types are changed to the
> > new scheme.
> >
> > Since we're touching them anyway, we change get and set to take a
> > const struct kernel_param (which they were, and will be again).
> >
> > To reduce churn, module_param_call creates the ops struct so the callers
> > don't have to change (and casts the functions to reduce warnings).
> > The modern version which takes an ops struct is called module_param_cb.
>
> This is nice, as it also reduces the size of struct kernel_param, so
> each parameter uses less footprint (who cares, though?) :)
>
> But, just wondering whether we still need to export get/set
> functions. They can be called from ops now, so if any, it can be
> defined even as an inlinefunction or a macro.
My thought too, so I tried that, but many are still used like so:
module_param_call(foo, set_foo, param_get_uint, NULL, 0644);
They can all be replaced in time with something like:
static int param_get_foo(char *buffer, const struct kernel_param *kp)
{
return param_ops_uint.get(buffer, kp);
}
But it'll take a transition period.
Thanks!
Rusty.
>
>
> thanks,
>
> Takashi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: pull request: wireless-next-2.6 2009-10-28
From: Jarek Poplawski @ 2009-10-30 11:06 UTC (permalink / raw)
To: Johannes Berg
Cc: Bartlomiej Zolnierkiewicz, Pekka Enberg, David Miller,
linux-wireless, netdev, linux-kernel, linville
In-Reply-To: <1256886023.3555.5.camel@johannes.local>
On 30-10-2009 08:00, Johannes Berg wrote:
> On Thu, 2009-10-29 at 22:48 +0100, Bartlomiej Zolnierkiewicz wrote:
>
>> I don't think that my technical arguments are pointless.
>>
>> Quite the contrary, I'm pretty confident that addressing my review concerns
>> would result in better RT28x00 / RT30x0 support in the very near future.
>
> So you know how to make the driver better, whine that it doesn't work,
> but don't help out either. Hey, good way to collaborate!
>
>>> It should be pretty obvious by now that the best way to improve things
>>> is to work with the relevant maintainers, not against them. (Unless
>>> you wish your work to be ignored, of course.)
>> I work with a lot of other maintainers. I would say that providing valuable
>> review feedback is also "working with" (at least I would be very happy with
>> such feedback in my projects).
>
> Face it though, people have read your feedback, thought about it, and
> decided to disagree. You seem to have a childish problem with that,
> appealing to higher authority to get your way just makes you look more
> childish.
There are various ways to disagree, and ignoring by John questions
from a merited developer both in this referenced lkml and current
threads looks at least strange (if not offensive) as well.
Jarek P.
^ permalink raw reply
* [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
From: William Allen Simpson @ 2009-10-30 11:00 UTC (permalink / raw)
To: Linux Kernel Network Developers
[-- Attachment #1: Type: text/plain, Size: 437 bytes --]
First of all, this is my first attempt at locks, so I'd like early review.
Secondly, scripts/checkpatch.pl tells me:
ERROR: do not initialise statics to 0 or NULL
#95: FILE: net/ipv4/tcp.c:2977:
+static struct tcp_cookie_secret *tcp_secret_generating = NULL;
They need to be NULL, and I'm not planning on exporting them, so what's
the preferred mechanism?
(I've grep'd many other instances of statics = 0 or NULL, so I'm not alone.)
[-- Attachment #2: TCPCT-1d.patch --]
[-- Type: text/plain, Size: 6070 bytes --]
diff --git a/include/linux/cryptohash.h b/include/linux/cryptohash.h
index c118b2a..ec78a4b 100644
--- a/include/linux/cryptohash.h
+++ b/include/linux/cryptohash.h
@@ -2,6 +2,7 @@
#define __CRYPTOHASH_H
#define SHA_DIGEST_WORDS 5
+#define SHA_MESSAGE_BYTES (512 /*bits*/ / 8)
#define SHA_WORKSPACE_WORDS 80
void sha_init(__u32 *buf);
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 51b7426..f669c43 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1526,12 +1526,18 @@ static inline int tcp_s_data_size(const struct tcp_sock *tp)
: 0;
}
+/* Using SHA1 for now, define some constants.
+ */
+#define COOKIE_DIGEST_WORDS (SHA_DIGEST_WORDS)
+#define COOKIE_MESSAGE_WORDS (SHA_MESSAGE_BYTES / 4)
+#define COOKIE_WORKSPACE_WORDS (COOKIE_DIGEST_WORDS + COOKIE_MESSAGE_WORDS)
+
/* As tcp_request_sock has already been extended in other places, the
* only remaining method is to pass stack values along as function
* parameters. These parameters are not needed after sending SYNACK.
*/
struct tcp_extend_values {
- u8 cookie_bakery[TCP_COOKIE_MAX];
+ u32 cookie_bakery[COOKIE_WORKSPACE_WORDS];
u8 cookie_plus;
u8 cookie_in_always:1,
cookie_out_never:1;
@@ -1542,6 +1548,8 @@ static inline struct tcp_extend_values *tcp_xv(const struct request_values *rvp)
return (struct tcp_extend_values *)rvp;
}
+extern int tcp_cookie_generator(struct tcp_extend_values *xvp);
+
extern void tcp_v4_init(void);
extern void tcp_init(void);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 12409df..32f8711 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2933,6 +2933,126 @@ EXPORT_SYMBOL(tcp_md5_hash_key);
#endif
+/**
+ * Each Responder maintains up to two secret values concurrently for
+ * efficient secret rollover. Each secret value has 4 states:
+ *
+ * Generating.
+ * Generates new Responder-Cookies, but not yet used for primary
+ * verification. This is a short-term state, typically lasting only
+ * one round trip time (RTT).
+ *
+ * Primary.
+ * Used both for generation and primary verification.
+ *
+ * Retiring.
+ * Used for verification, until the first failure that can be
+ * verified by the newer Generating secret. At that time, this
+ * cookie's state is changed to Secondary, and the Generating
+ * cookie's state is changed to Primary. This is a short-term state,
+ * typically lasting only one round trip time (RTT).
+ *
+ * Secondary.
+ * Used for secondary verification, after primary verification
+ * failures. This state lasts no more than twice the Maximum Segment
+ * Lifetime (2MSL). Then, the secret is discarded.
+ */
+struct tcp_cookie_secret {
+ /* The secret is divided into two parts. The digest part is the
+ * equivalent of previously hashing a secret and saving the state,
+ * and serves as an initialization vector (IV). The message part
+ * serves as the trailing secret.
+ */
+ u32 secrets[COOKIE_WORKSPACE_WORDS];
+ unsigned long expires;
+};
+
+#define TCP_SECRET_1MSL (HZ * TCP_PAWS_MSL)
+#define TCP_SECRET_2MSL (HZ * TCP_PAWS_MSL * 2)
+#define TCP_SECRET_LIFE (HZ * 600)
+
+static struct tcp_cookie_secret tcp_secret_one;
+static struct tcp_cookie_secret tcp_secret_two;
+
+static struct tcp_cookie_secret *tcp_secret_generating = NULL;
+static struct tcp_cookie_secret *tcp_secret_primary = NULL;
+static struct tcp_cookie_secret *tcp_secret_retiring = NULL;
+static struct tcp_cookie_secret *tcp_secret_secondary = NULL;
+
+static DEFINE_RWLOCK(tcp_secret_locker);
+
+/* Fill cookie_bakery with current generator.
+ * Returns: 0 o success.
+ */
+int tcp_cookie_generator(struct tcp_extend_values *xvp)
+{
+ u32 secrets[COOKIE_WORKSPACE_WORDS];
+
+ if (unlikely(NULL == tcp_secret_primary)) {
+ get_random_bytes(secrets, sizeof(secrets));
+
+ /* The first time, paranoia assumes that the randomization
+ * function isn't as strong. But this secret initialization
+ * is delayed until the last possible moment (packet arrival).
+ * Although that time is observable, it is unpredictably
+ * variable. Mash in the fastest clock bits available, and
+ * expire the secret extra quickly.
+ */
+ secrets[COOKIE_DIGEST_WORDS] ^= (u32)jiffies;
+
+ write_lock(&tcp_secret_locker);
+ if (NULL == tcp_secret_primary) {
+ /* still needs initialization */
+ memcpy((u8 *)&tcp_secret_one.secrets[0],
+ (u8 *)&secrets[0],
+ sizeof(secrets));
+ tcp_secret_one.expires = jiffies + TCP_SECRET_1MSL;
+ tcp_secret_primary = &tcp_secret_one;
+ /* unused at this time */
+ memcpy((u8 *)&tcp_secret_two.secrets[0],
+ (u8 *)&secrets[0],
+ sizeof(secrets));
+ tcp_secret_two.expires = jiffies; /* past due */
+ tcp_secret_secondary = &tcp_secret_two;
+ }
+ write_unlock(&tcp_secret_locker);
+ }
+
+ if (unlikely(time_after(jiffies, tcp_secret_primary->expires))) {
+ get_random_bytes(secrets, sizeof(secrets));
+
+ write_lock(&tcp_secret_locker);
+ if (time_after(jiffies, tcp_secret_primary->expires)) {
+ /* still needs refreshing */
+ tcp_secret_primary->expires = jiffies
+ + TCP_SECRET_2MSL;
+ tcp_secret_retiring = tcp_secret_primary;
+ /* new generator at secondary position */
+ memcpy((u8 *)&tcp_secret_secondary->secrets[0],
+ (u8 *)&secrets[0],
+ sizeof(secrets));
+ tcp_secret_secondary->expires = jiffies
+ + TCP_SECRET_LIFE;
+ tcp_secret_generating = tcp_secret_secondary;
+ }
+ write_unlock(&tcp_secret_locker);
+ }
+
+ read_lock(&tcp_secret_locker);
+ if (unlikely(NULL != tcp_secret_generating)) {
+ memcpy((u8 *)&xvp->cookie_bakery[0],
+ (u8 *)&tcp_secret_generating->secrets[0],
+ sizeof(tcp_secret_generating->secrets));
+ } else {
+ memcpy((u8 *)&xvp->cookie_bakery[0],
+ (u8 *)&tcp_secret_primary->secrets[0],
+ sizeof(tcp_secret_primary->secrets));
+ }
+ read_unlock(&tcp_secret_locker);
+ return 0;
+}
+EXPORT_SYMBOL(tcp_cookie_generator);
+
void tcp_done(struct sock *sk)
{
if (sk->sk_state == TCP_SYN_SENT || sk->sk_state == TCP_SYN_RECV)
^ permalink raw reply related
* Re: [PATCH 2/3] net: TCP thin linear timeouts
From: apetlund @ 2009-10-30 10:48 UTC (permalink / raw)
To: Rick Jones
Cc: Andreas Petlund, Ilpo Järvinen, Arnd Hannemann, Eric Dumazet,
Netdev, LKML, shemminger, David Miller
> Just how thin can a thin stream be when a thin stream is found thin? (to
the
> cadence of "How much wood could a woodchuck chuck if a woodchuck could
chuck wood?")
>
> Does a stream get so thin that a user's send could not be split into
four,
> sub-MSS TCP segments?
That was a nifty idea: Anti-Nagle the segments to be able to trigger fast
retransmissions. I think it is possible.
Besides using more resources on each send, this scheme will introduce the
need to delay parts of the segment, which is undesirable for
time-dependent applications (the intended target of the mechanisms).
I think it would be fun to implement and play around with such a mechanism
to see the effects.
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH 0/6] Bonding simplifications and netns support
From: Eric W. Biederman @ 2009-10-30 10:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev, fubar
In-Reply-To: <m11vkltj4z.fsf@fess.ebiederm.org>
ebiederm@xmission.com (Eric W. Biederman) writes:
> David Miller <davem@davemloft.net> writes:
>
>> From: ebiederm@xmission.com (Eric W. Biederman)
>> Date: Thu, 29 Oct 2009 17:16:54 -0700
>>
>>> I recently had it pointed out to me that the bonding driver does not
>>> work in a network namespace. So I have simplified the bonding driver
>>> a bit, added support for ip link add and ip link del, and finally made
>>> the bonding driver work in multiple network namespaces.
>>>
>>> The most note worthy change in the patchset is the addition of support
>>> in the networking core for registering a sysfs group for a device.
>>>
>>> Using this in the bonding driver simplifies the code and removes a
>>> userspace race between actions triggered by the netlink event and the
>>> bonding sysfs attributes appearing.
>>
>> I have no objections to these patches, but I'd like the bonding
>> folks to have a chance to look at it before I apply to net-next-2.6
>
> Sure.
>
>> One question though, are you sure this clever extra slot scheme
>> in patch #1 works for, f.e., a bond of wireless devices? It seems
>> like it would work out, but I wanted to ask to make sure you
>> considered that case.
>
> I have not explicitly tested wireless devices. But I did make certain
> we have enough slots in the array. I did write the code so that a
> device driver can use at most one slot (the next slot gets
> unconditionally stomped). Other that it is just shifting of where
> sysfs_create_group and sysfs_remove_group are called. So I would
> be totally stunned if bonded wireless devices started failing from
> this change.
Bah. The argument is better than that. The bond_group that I am
messing with only applies to the virtual bonding devices. The virtual
bond device is never a wireless device. So we will never see in
practice all three groups on the same network device.
Eric
^ permalink raw reply
* Re: [PATCH 2/8] param: use ops in struct kernel_param, rather than get and set fns directly
From: Takashi Iwai @ 2009-10-30 10:18 UTC (permalink / raw)
To: Rusty Russell
Cc: linux-kernel, David S. Miller, Ville Syrjala, Dmitry Torokhov,
Alessandro Rubini, Michal Januszewski, Trond Myklebust,
J. Bruce Fields, Neil Brown, linux-input, linux-fbdev-devel,
linux-nfs, netdev
In-Reply-To: <20091022142337.EC4A85362F@mx1.suse.de>
At Fri, 23 Oct 2009 00:51:28 +1030,
Rusty Russell wrote:
>
> This is more kernel-ish, saves some space, and also allows us to
> expand the ops without breaking all the callers who are happy for the
> new members to be NULL.
>
> The few places which defined their own param types are changed to the
> new scheme.
>
> Since we're touching them anyway, we change get and set to take a
> const struct kernel_param (which they were, and will be again).
>
> To reduce churn, module_param_call creates the ops struct so the callers
> don't have to change (and casts the functions to reduce warnings).
> The modern version which takes an ops struct is called module_param_cb.
This is nice, as it also reduces the size of struct kernel_param, so
each parameter uses less footprint (who cares, though?) :)
But, just wondering whether we still need to export get/set
functions. They can be called from ops now, so if any, it can be
defined even as an inlinefunction or a macro.
thanks,
Takashi
^ permalink raw reply
* Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
From: Patrick McHardy @ 2009-10-30 10:08 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <m1skd1qjzb.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
>
>>> As for rtnl_link_register it always succeeds so let's just
>>> remove the return code and call it good.
>> You need unroll anyways for the other failure conditions, so
>> why not simply add an err1/2 and be safe for future changes?
>
> Not a real problem. I was just thinking of things like the
> dummy driver that have this same issue and the fact that since
> rtnl_link_register never fails we never test the error path.
> So it would be much less error prone and less code to remove
> the possibility of rtnl_link_register failing.
Mhh good point, I think I added the broken dummy code myself :)
The main reason for not returning void from rtnl_link_register()
was so new drivers that are written with rtnl_link in mind from
the beginning (and thus usually don't do anything like default
device creation, sysfs registrations etc.) can simply do
"return rtnl_link_register(&ops)" in their init function. But
that's admittedly not a very strong argument :)
^ permalink raw reply
* Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
From: Eric W. Biederman @ 2009-10-30 10:00 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <4AEAB2F1.5070200@trash.net>
Patrick McHardy <kaber@trash.net> writes:
> Eric W. Biederman wrote:
>> Patrick McHardy <kaber@trash.net> writes:
>>
>>> Eric W. Biederman wrote:
>>>> +static struct rtnl_link_ops bond_link_ops __read_mostly = {
>>>> + .kind = "bond",
>>>> + .setup = bond_setup,
>>>> + .validate = bond_validate,
>>>> +};
>>> One more thing - you need to initialize .priv_size here so
>>> the devices created through rtnl_link have enough private
>>> room allocated.
>>
>> Wow and the code works when I test it without that ouch!
>>
>> As for rtnl_link_register it always succeeds so let's just
>> remove the return code and call it good.
>
> You need unroll anyways for the other failure conditions, so
> why not simply add an err1/2 and be safe for future changes?
Not a real problem. I was just thinking of things like the
dummy driver that have this same issue and the fact that since
rtnl_link_register never fails we never test the error path.
So it would be much less error prone and less code to remove
the possibility of rtnl_link_register failing.
Eric
^ permalink raw reply
* Re: [RFC] multiqueue changes
From: Jarek Poplawski @ 2009-10-30 10:00 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Eric Dumazet, David S. Miller, Linux Netdev List
In-Reply-To: <4AEA1357.3090307@trash.net>
On Thu, Oct 29, 2009 at 11:12:39PM +0100, Patrick McHardy wrote:
> Jarek Poplawski wrote:
> > On Thu, Oct 29, 2009 at 05:37:23PM +0100, Patrick McHardy wrote:
> > ...
> >> Well, we do need both values for supporting changes to the actually
> >> used numbers of TX queues. If I understood Dave's explanation correctly,
> >> this is also what's intended. It also doesn't seem unreasonable
> >> what bnx2 is doing.
> >
> > Exactly. With a growing number of cores, both available and powered
> > off, these values will be soon treated more carefully than now.
> >
> >> But getting back to the problem Eric reported - so you're suggesting
> >> that bnx2.c should also adjust num_tx_queues in case the hardware
> >> doesn't support multiqueue? That seems reasonable as well.
> >
> > Currently, declaring num_tx_queues with alloc_netdev_mq() looks like
> > too soon for some drivers. It seems they should be able to do it
> > separately later during the .probe.
>
> The value passed into alloc_netdev_mq() is just used for allocation
> purposes, from what I can tell there's no downside in reducing it
> before the dev_activate() call.
Right, but IMHO this reducing (or reallocation) should be done with
some API. Simple overwriting of num_tx_queues proposed by Eric, even
if no downside now, seems to be asking for problems in the future.
> > There is a question if .ndo_open should be considered too.
>
> I currently can't see any purpose in decreasing num_tx_queues after
> registration instead of real_num_tx_queues.
I agree, but since Eric's example shows some drivers do it (almost)
like this, I'd prefer authors/maintainers answer this question.
Jarek P.
^ permalink raw reply
* [PATCH 7/6] bond: Get the rtnl_link_ops support correct
From: Eric W. Biederman @ 2009-10-30 9:58 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <4AEAB2F1.5070200@trash.net>
- Don't call rtnl_link_unregister if rtnl_link_register fails
- Set .priv_size so we aren't stomping on uninitialized memory
when we use netdev_priv, on bond devices created with
ip link add type bond.
Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
---
drivers/net/bonding/bond_main.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fed44e6..0ef051c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5004,6 +5004,7 @@ static int bond_validate(struct nlattr *tb[], struct nlattr *data[])
static struct rtnl_link_ops bond_link_ops __read_mostly = {
.kind = "bond",
+ .priv_size = sizeof(struct bonding),
.setup = bond_setup,
.validate = bond_validate,
};
@@ -5105,7 +5106,7 @@ static int __init bonding_init(void)
res = rtnl_link_register(&bond_link_ops);
if (res)
- goto err;
+ goto err_link;
for (i = 0; i < max_bonds; i++) {
res = bond_create(&init_net, NULL);
@@ -5124,6 +5125,7 @@ out:
return res;
err:
rtnl_link_unregister(&bond_link_ops);
+err_link:
unregister_pernet_gen_subsys(bond_net_id, &bond_net_ops);
goto out;
--
1.6.3.1.54.g99dd.dirty
^ permalink raw reply related
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Eric Dumazet @ 2009-10-30 9:41 UTC (permalink / raw)
To: Francis Moreau
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <38b2ab8a0910300144i7a3c190fi9aa3d079c9cdb754@mail.gmail.com>
Francis Moreau a écrit :
> Hello Eric,
>
> It seems I still have a related bug, please have a look to the following oops.
>
> This happened on a 2.6.32-rc5 where your patch is included.
>
> [107304.558821] nfsd: last server has exited, flushing export cache
> [107304.558848] ------------[ cut here ]------------
> [107304.558858] WARNING: at net/ipv4/af_inet.c:153
> inet_sock_destruct+0x161/0x17c()
> [107304.558862] Hardware name: P5K-VM
> [107304.558865] Modules linked in: kvm_intel kvm jfs loop nfsd lockd
> nfs_acl auth_rpcgss exportfs sunrpc [last unloaded: microcode]
> [107304.558889] Pid: 8198, comm: nfsd Tainted: G M 2.6.32-rc5 #25
> [107304.558892] Call Trace:
> [107304.558899] [<ffffffff81429f19>] ? inet_sock_destruct+0x161/0x17c
> [107304.558907] [<ffffffff810487e9>] warn_slowpath_common+0x7c/0xa9
> [107304.558914] [<ffffffff8104882a>] warn_slowpath_null+0x14/0x16
> [107304.558920] [<ffffffff81429f19>] inet_sock_destruct+0x161/0x17c
> [107304.558927] [<ffffffff813c8741>] __sk_free+0x23/0xe7
> [107304.558933] [<ffffffff813c8881>] sk_free+0x1f/0x21
> [107304.558939] [<ffffffff813c894b>] sk_common_release+0xc8/0xcd
> [107304.558944] [<ffffffff81420b59>] udp_lib_close+0xe/0x10
> [107304.558951] [<ffffffff814299bf>] inet_release+0x55/0x5c
> [107304.558957] [<ffffffff813c5aa9>] sock_release+0x1f/0x71
> [107304.558962] [<ffffffff813c5b22>] sock_close+0x27/0x2b
> [107304.558968] [<ffffffff810eb60f>] __fput+0xfb/0x1c0
> [107304.558973] [<ffffffff810eb6f1>] fput+0x1d/0x1f
> [107304.558995] [<ffffffffa0013e23>] svc_sock_free+0x40/0x56 [sunrpc]
> [107304.559018] [<ffffffffa001f392>] svc_xprt_free+0x43/0x53 [sunrpc]
> [107304.559038] [<ffffffffa001f34f>] ? svc_xprt_free+0x0/0x53 [sunrpc]
> [107304.559048] [<ffffffff811d9275>] kref_put+0x43/0x4f
> [107304.559069] [<ffffffffa001e67a>] svc_close_xprt+0x55/0x5e [sunrpc]
> [107304.559088] [<ffffffffa001e6d3>] svc_close_all+0x50/0x69 [sunrpc]
> [107304.559107] [<ffffffffa0012a2b>] svc_destroy+0x9e/0x142 [sunrpc]
> [107304.559126] [<ffffffffa0012b88>] svc_exit_thread+0xb9/0xc2 [sunrpc]
> [107304.559138] [<ffffffffa008981b>] ? nfsd+0x0/0x13f [nfsd]
> [107304.559149] [<ffffffffa0089940>] nfsd+0x125/0x13f [nfsd]
> [107304.559157] [<ffffffff810685e3>] kthread+0x82/0x8a
> [107304.559164] [<ffffffff8100c13a>] child_rip+0xa/0x20
> [107304.559172] [<ffffffff8100baad>] ? restore_args+0x0/0x30
> [107304.559179] [<ffffffff81068561>] ? kthread+0x0/0x8a
> [107304.559185] [<ffffffff8100c130>] ? child_rip+0x0/0x20
> [107304.559191] ---[ end trace c107131f4762168c ]---
> [107304.927931] NFSD: Using /var/lib/nfs/v4recovery as the NFSv4 state
> recovery directory
> [107304.932765] NFSD: starting 90-second grace period
>
Thanks Francis, I think I found the problem.
I am preparing a patch, test it, and submit it in couple of hours
^ permalink raw reply
* Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
From: Patrick McHardy @ 2009-10-30 9:33 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <m1eiols0a4.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
> Patrick McHardy <kaber@trash.net> writes:
>
>> Eric W. Biederman wrote:
>>> +static struct rtnl_link_ops bond_link_ops __read_mostly = {
>>> + .kind = "bond",
>>> + .setup = bond_setup,
>>> + .validate = bond_validate,
>>> +};
>> One more thing - you need to initialize .priv_size here so
>> the devices created through rtnl_link have enough private
>> room allocated.
>
> Wow and the code works when I test it without that ouch!
>
> As for rtnl_link_register it always succeeds so let's just
> remove the return code and call it good.
You need unroll anyways for the other failure conditions, so
why not simply add an err1/2 and be safe for future changes?
^ permalink raw reply
* Re: [PATCH 5/6] bond: Implement a basic set of rtnl link ops
From: Eric W. Biederman @ 2009-10-30 9:23 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, netdev, Jay Vosburgh, Eric W. Biederman
In-Reply-To: <4AEAA403.7080204@trash.net>
Patrick McHardy <kaber@trash.net> writes:
> Eric W. Biederman wrote:
>> +static struct rtnl_link_ops bond_link_ops __read_mostly = {
>> + .kind = "bond",
>> + .setup = bond_setup,
>> + .validate = bond_validate,
>> +};
>
> One more thing - you need to initialize .priv_size here so
> the devices created through rtnl_link have enough private
> room allocated.
Wow and the code works when I test it without that ouch!
As for rtnl_link_register it always succeeds so let's just
remove the return code and call it good.
Eric
^ permalink raw reply
* [RFC, PATCH] net: suspicious test in dev_change_name()
From: Eric Dumazet @ 2009-10-30 9:20 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List
While preparing a patch for net-next-2.6, I noticed following code in dev_change_name()
int err = 0;
...
ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
ret = notifier_to_errno(ret);
if (ret) {
<< HERE >> if (err) {
printk(KERN_ERR
"%s: name change rollback failed: %d.\n",
dev->name, ret);
} else {
err = ret;
memcpy(dev->name, oldname, IFNAMSIZ);
goto rollback;
}
}
It seems intent was to test if notifier_to_errno() was null ?
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/core/dev.c b/net/core/dev.c
index b8f74cf..029cd41 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -939,9 +939,9 @@ rollback:
write_unlock_bh(&dev_base_lock);
ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
- ret = notifier_to_errno(ret);
if (ret) {
+ err = notifier_to_errno(ret);
if (err) {
printk(KERN_ERR
"%s: name change rollback failed: %d.\n",
^ permalink raw reply related
* Re: [PATCH v3 4/7] fsl_pq_mdio: Add Suport for etsec2.0 devices.
From: David Miller @ 2009-10-30 8:44 UTC (permalink / raw)
To: B05799; +Cc: netdev, afleming
In-Reply-To: <9F4C7D19E8361D4C94921B95BE08B81B9503DF@zin33exm22.fsl.freescale.net>
From: "Kumar Gopalpet-B05799" <B05799@freescale.com>
Date: Fri, 30 Oct 2009 14:12:22 +0530
> We were referring to the logs shown on the gitweb.
> The tree log does not show this commit.
> (search for this commit id failed)
> http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fdavem%2Fnet-next-2.6.git
> &a=search&h=29906f6a427d2004a515ebbcdc7b28bae8f6c19c&st=commit&s=e72701a
> cbe0b35e52d3f04d442837c06b4e64f1c
>
> Whereas, the log for the file does show the commit.
> http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=histo
> ry;f=drivers/net/fsl_pq_mdio.c;h=6ac4648669728421b934d32646ffbb626128387
> 6;hb=HEAD
>
> Why is there such a mismatch on the tree itself?
If you are using HTTP for accessing my GIT tree(s) that's your
problem.
I do not update the web based GIT information when I push into my
tree(s), only the GIT and SSH protocols are reliable ways to clone and
access my tree(s).
^ permalink raw reply
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Francis Moreau @ 2009-10-30 8:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <4AC1D0F5.4050709@gmail.com>
Hello Eric,
It seems I still have a related bug, please have a look to the following oops.
This happened on a 2.6.32-rc5 where your patch is included.
[107304.558821] nfsd: last server has exited, flushing export cache
[107304.558848] ------------[ cut here ]------------
[107304.558858] WARNING: at net/ipv4/af_inet.c:153
inet_sock_destruct+0x161/0x17c()
[107304.558862] Hardware name: P5K-VM
[107304.558865] Modules linked in: kvm_intel kvm jfs loop nfsd lockd
nfs_acl auth_rpcgss exportfs sunrpc [last unloaded: microcode]
[107304.558889] Pid: 8198, comm: nfsd Tainted: G M 2.6.32-rc5 #25
[107304.558892] Call Trace:
[107304.558899] [<ffffffff81429f19>] ? inet_sock_destruct+0x161/0x17c
[107304.558907] [<ffffffff810487e9>] warn_slowpath_common+0x7c/0xa9
[107304.558914] [<ffffffff8104882a>] warn_slowpath_null+0x14/0x16
[107304.558920] [<ffffffff81429f19>] inet_sock_destruct+0x161/0x17c
[107304.558927] [<ffffffff813c8741>] __sk_free+0x23/0xe7
[107304.558933] [<ffffffff813c8881>] sk_free+0x1f/0x21
[107304.558939] [<ffffffff813c894b>] sk_common_release+0xc8/0xcd
[107304.558944] [<ffffffff81420b59>] udp_lib_close+0xe/0x10
[107304.558951] [<ffffffff814299bf>] inet_release+0x55/0x5c
[107304.558957] [<ffffffff813c5aa9>] sock_release+0x1f/0x71
[107304.558962] [<ffffffff813c5b22>] sock_close+0x27/0x2b
[107304.558968] [<ffffffff810eb60f>] __fput+0xfb/0x1c0
[107304.558973] [<ffffffff810eb6f1>] fput+0x1d/0x1f
[107304.558995] [<ffffffffa0013e23>] svc_sock_free+0x40/0x56 [sunrpc]
[107304.559018] [<ffffffffa001f392>] svc_xprt_free+0x43/0x53 [sunrpc]
[107304.559038] [<ffffffffa001f34f>] ? svc_xprt_free+0x0/0x53 [sunrpc]
[107304.559048] [<ffffffff811d9275>] kref_put+0x43/0x4f
[107304.559069] [<ffffffffa001e67a>] svc_close_xprt+0x55/0x5e [sunrpc]
[107304.559088] [<ffffffffa001e6d3>] svc_close_all+0x50/0x69 [sunrpc]
[107304.559107] [<ffffffffa0012a2b>] svc_destroy+0x9e/0x142 [sunrpc]
[107304.559126] [<ffffffffa0012b88>] svc_exit_thread+0xb9/0xc2 [sunrpc]
[107304.559138] [<ffffffffa008981b>] ? nfsd+0x0/0x13f [nfsd]
[107304.559149] [<ffffffffa0089940>] nfsd+0x125/0x13f [nfsd]
[107304.559157] [<ffffffff810685e3>] kthread+0x82/0x8a
[107304.559164] [<ffffffff8100c13a>] child_rip+0xa/0x20
[107304.559172] [<ffffffff8100baad>] ? restore_args+0x0/0x30
[107304.559179] [<ffffffff81068561>] ? kthread+0x0/0x8a
[107304.559185] [<ffffffff8100c130>] ? child_rip+0x0/0x20
[107304.559191] ---[ end trace c107131f4762168c ]---
[107304.927931] NFSD: Using /var/lib/nfs/v4recovery as the NFSv4 state
recovery directory
[107304.932765] NFSD: starting 90-second grace period
--
Francis
^ permalink raw reply
* RE: [PATCH v3 4/7] fsl_pq_mdio: Add Suport for etsec2.0 devices.
From: Kumar Gopalpet-B05799 @ 2009-10-30 8:42 UTC (permalink / raw)
To: netdev; +Cc: Fleming Andy-AFLEMING, David Miller
In-Reply-To: <20091030.005924.56385149.davem@davemloft.net>
>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: Friday, October 30, 2009 1:29 PM
>To: Kumar Gopalpet-B05799
>Cc: netdev@vger.kernel.org; Fleming Andy-AFLEMING
>Subject: Re: [PATCH v3 4/7] fsl_pq_mdio: Add Suport for
>etsec2.0 devices.
>
>From: "Kumar Gopalpet-B05799" <B05799@freescale.com>
>Date: Fri, 30 Oct 2009 12:52:29 +0530
>
>>
>> 1. The patch failure is due to the absence of following commit in
>> net-next-2.6 tree.
>>
>> commit e72701acbe0b35e52d3f04d442837c06b4e64f1c
>> Author: Anton Vorontsov <avorontsov@ru.mvista.com>
>> Date: Wed Oct 14 14:54:52 2009 -0700
>
>It is in the net-next-2.6 tree, something is wrong with your clone.
We were referring to the logs shown on the gitweb.
The tree log does not show this commit.
(search for this commit id failed)
http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fdavem%2Fnet-next-2.6.git
&a=search&h=29906f6a427d2004a515ebbcdc7b28bae8f6c19c&st=commit&s=e72701a
cbe0b35e52d3f04d442837c06b4e64f1c
Whereas, the log for the file does show the commit.
http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git;a=histo
ry;f=drivers/net/fsl_pq_mdio.c;h=6ac4648669728421b934d32646ffbb626128387
6;hb=HEAD
Why is there such a mismatch on the tree itself?
>
>davem@sunset:~/src/GIT/net-next-2.6$ git describe
>e72701acbe0b35e52d3f04d442837c06b4e64f1c
>v2.6.31-9041-ge72701a
>davem@sunset:~/src/GIT/net-next-2.6$
Even we get this on our clone.
However, the changes to the file itself are absent.
Shall we generate the patch series against the snapshot (which contains
the changes)?
--
Thanks
Sandeep
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: use hlist_for_each_entry()
From: David Miller @ 2009-10-30 8:40 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <4AEAA5DD.3040704@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 30 Oct 2009 09:37:49 +0100
> Small cleanup of __dev_get_by_name() and __dev_get_by_index()
> to use hlist_for_each_entry() : They'll look like their _rcu variant.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ 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