* 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: 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: [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: 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
* [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 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
* 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: [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 0/5] Candidate fix for increased number of GFP_ATOMIC failures V2
From: Karol Lewandowski @ 2009-10-30 14:23 UTC (permalink / raw)
To: Mel Gorman
Cc: Karol Lewandowski, Mel LKML, Frans Pop, Jiri Kosina, Sven Geggus,
Tobias Oetiker, Rafael J. Wysocki, David Miller, Reinette Chatre,
Kalle Valo, David Rientjes, KOSAKI Motohiro, Mohamed Abbas,
Jens Axboe, John W. Linville, Pekka Enberg,
Bartlomiej Zolnierkiewicz, Greg Kroah-Hartman,
Stephan von Krawczynski, Kernel Testers List,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
"linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org"
In-Reply-To: <20091028115926.GW8900-wPRd99KPJ+uzQB+pC5nmwQ@public.gmane.org>
On Wed, Oct 28, 2009 at 11:59:26AM +0000, Mel Gorman wrote:
> On Wed, Oct 28, 2009 at 12:42:08PM +0100, Karol Lewandowski wrote:
> > On Sat, Oct 24, 2009 at 02:46:56PM +0100, Mel LKML wrote:
> > I've tested patches 1+2+3+4 in my normal usage scenario (do some work,
> > suspend, do work, suspend, ...) and it failed today after 4 days (== 4
> > suspend-resume cycles).
> >
> > I'll test 1-5 now.
2.6.32-rc5 with patches 1-5 fails too.
> Also, what was the behaviour of the e100 driver when suspending before
> this commit?
>
> 6905b1f1a03a48dcf115a2927f7b87dba8d5e566: Net / e100: Fix suspend of devices that cannot be power managed
This was discussed before with e100 maintainers and Rafael. Reverting
this patch didn't change anything.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Benjamin LaHaise @ 2009-10-30 14:35 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Eric Dumazet, Octavian Purdila, netdev, Cosmin Ratiu
In-Reply-To: <m1vdhxu01v.fsf@fess.ebiederm.org>
On Thu, Oct 29, 2009 at 06:45:32PM -0700, Eric W. Biederman wrote:
> The reason for the existence of sysfs_dirent is as things grow larger
> we want to keep the amount of RAM consumed down. So we don't pin
> everything in the dcache. So we try and keep the amount of memory
> consumed down.
I'm aware of that, but for users running into this sort of scaling issue,
the amount of RAM required is a non-issue (30,000 interfaces require about
1GB of RAM at present), making the question more one of how to avoid the
overhead for users who don't require it. I'd prefer a config option. The
only way I can really see saving memory usage is to somehow tie sysfs dirent
lookups into the network stack's own tables for looking up device entries.
The network stack already has to cope with this kind of scaling, and that
would save the RAM.
> So I would like to see how much we can par down.
> For dealing with seeks in the middle of readdir I expect the best way
> to do that is to be inspired by htrees in extNfs and return a hash of
> the filename as our position, and keep the filename list sorted by
> that hash. Since we are optimizing for size we don't need to store
> that hash. Then we can turn that list into a some flavor of sorted
> binary tree.
readdir() generally isn't an issue at present.
> I'm surprised sysfs_count_nlink shows up, as it is not directly on the
> add or remove path. I think the answer there is to change s_flags
> into a set of bitfields and make link_count one of them, perhaps
> 16bits long. If we ever overflow our bitfield we can just set link
> count to 0, and userspace (aka find) will know it can't optimized
> based on link count.
It shows up because of the bits of userspace (udev) touching the directory
from things like the hotplug code path.
> I was expecting someone to run into problems with the linear directory
> of sysfs someday.
Alas, sysfs isn't the only offender.
-ben
^ permalink raw reply
* Re: [PATCH] net: allow netdev_wait_allrefs() to run faster
From: Eric Dumazet @ 2009-10-30 14:43 UTC (permalink / raw)
To: Benjamin LaHaise
Cc: Eric W. Biederman, Octavian Purdila, netdev, Cosmin Ratiu
In-Reply-To: <20091030143527.GA3141@kvack.org>
Benjamin LaHaise a écrit :
> On Thu, Oct 29, 2009 at 06:45:32PM -0700, Eric W. Biederman wrote:
>> I was expecting someone to run into problems with the linear directory
>> of sysfs someday.
>
> Alas, sysfs isn't the only offender.
>
> -ben
In my tests, the sysfs lookup by name is the big offender, I believe you should
post your rb_tree patch ASAP ;) Then we can go further
^ permalink raw reply
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Eric Dumazet @ 2009-10-30 14:55 UTC (permalink / raw)
To: Francis Moreau
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <38b2ab8a0910300641q78c08124o800a9dfc9486fba7@mail.gmail.com>
Francis Moreau a écrit :
> 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.
>
Please ignore/revert the last part of the patch (about net/sunrpc/xprtsock.c)
In xs_udp_data_ready() we really want to call skb_free_datagram(),
not skb_free_datagram_locked(), because socket is already locked.
Thanks
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] udev: create empty regular files to represent net interfaces
From: dann frazier @ 2009-10-30 15:00 UTC (permalink / raw)
To: Greg KH, Matt Domsch, Kay Sievers, linux-hotplug, Narendra_K,
netdev, Jordan_Ha
In-Reply-To: <20091030062249.GB5430@bongo.bofh.it>
On Fri, Oct 30, 2009 at 07:22:49AM +0100, Marco d'Itri wrote:
> On Oct 30, dann frazier <dannf@hp.com> wrote:
>
> > On Fri, Oct 30, 2009 at 04:30:29AM +0100, Marco d'Itri wrote:
> > > On Oct 29, dann frazier <dannf@dannf.org> wrote:
> > >
> > > > That would make a lot of users much happier (myself included), but it
> > > > does restrict us into one view. At different times, admins think of
> > > > their NICs by different properties. I may want to do IP assignment by
> > > [Citation needed]
> > Is "I" not clear, or do you just not believe I admin servers?
> I was referring to "At different times, admins think of their NICs by
> different properties", which I am not sure is a valid generalization.
Fair enough, I can't speak for others.
--
dann frazier
^ permalink raw reply
* Re: pull request: wireless-next-2.6 2009-10-28
From: John W. Linville @ 2009-10-30 15:02 UTC (permalink / raw)
To: Jarek Poplawski
Cc: Johannes Berg, Bartlomiej Zolnierkiewicz, Pekka Enberg,
David Miller, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20091030110616.GB6150-8HppEYmqbBCE+EvaaNYduQ@public.gmane.org>
On Fri, Oct 30, 2009 at 11:06:16AM +0000, Jarek Poplawski wrote:
> 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.
Did you read the thread for which Bartlomiej provided a link earlier?
There were ten responses (only three of them from him) in that thread.
His comments were not ignored, they were rejected.
Ever since Bartlomiej decided to tear himself away from
drivers/staging, he has been nothing but negative -- petty, whining,
indignat, whatever. Just what has he done to merit any special
consideration here? Why should he have any sort of veto over rt2x00?
And of all things on which to take a stand -- how dare the rt2x00 guys
use two header files instead of three? The nerve of those people!!!
Ridiculous...
John
--
John W. Linville Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org might be all we have. Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
From: Eric Dumazet @ 2009-10-30 15:03 UTC (permalink / raw)
To: Francis Moreau
Cc: Linux Kernel Mailing List, Linux Netdev List, David S. Miller
In-Reply-To: <4AEACD88.8080108@gmail.com>
Eric Dumazet a écrit :
> 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 ?
Here is the updated patch (against linux-2.6) , in case other people are interested to test it.
[PATCH take2] 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 +++++-----
5 files changed, 18 insertions(+), 12 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;
^ permalink raw reply related
* RE: [PATCH] udev: create empty regular files to represent net interfaces
From: Narendra_K @ 2009-10-30 15:13 UTC (permalink / raw)
To: dannf, greg
Cc: Matt_Domsch, kay.sievers, linux-hotplug, netdev, Jordan_Hargrave,
Charles_Rose, bhutchings
In-Reply-To: <20091029174600.GC3612@ldl.fc.hp.com>
>> This way the kernel has only one name, and so does userspace, and
>> everyone is happy.
>
>There are two issues, which really seem distinct to me.
>
>Users expect eth0 to map to first-onboard-nic. That's an
>installer issue (since the BIOS can already export this info)
>and I agree that if we want to "fix" that, we should fix it there.
>
I agree that installers have to be fixed in the sense that they can be
told to find the right interface. But, they expect determinism and
depend on "eth0 to map to first-onboard-nic". Installer is one of the
applications that is affected by this and needs user intervention, if it
is not told about the right interface. I discussed installer as it is so
much part of a user experience.
But the real issue is "eth0 does not map to first-onboard-nic" always
and applications expecting this would break in data center environments.
Both the solutions proposed provide a way to overcome it without
introducing state.
With regards,
Narendra K
^ permalink raw reply
* Re: pull request: wireless-next-2.6 2009-10-28
From: John W. Linville @ 2009-10-30 15:14 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: Pekka Enberg, David Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <200910292248.42229.bzolnier@gmail.com>
On Thu, Oct 29, 2009 at 10:48:42PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Thursday 29 October 2009 20:45:07 Pekka Enberg wrote:
> > I was simply pointing out that your time would probably be better
> > spent in improving the "proper" ralink wireless drivers but if you
>
> Thanks for the concern. However recent discussions made my realize how
> I should really be spending my time effectively way too well.
>
> > _really_ prefer to spend your time in pointless arguments, go ahead.
>
> I don't think that my technical arguments are pointless.
More like "petty"...
> Quite the contrary, I'm pretty confident that addressing my review concerns
> would result in better RT28x00 / RT30x0 support in the very near future.
Yeah, maybe. Or they could just waste someone's time mashing
together some definitions to prematurely turn two header files into
three instead of spending that time on resolving operational issues.
In either case it is a judgment call, and IMHO not your call to make.
> > 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).
"Valuable" feedback would be welcome. In fact, your initial feedback
in this case was welcome -- even moreso had it included a patch.
Your arrogant follow-ups and self-righteous indignation, however,
is quite a bit less welcome. Please take it somewhere else.
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: Connection tracking and vlan
From: Herbert Xu @ 2009-10-30 15:20 UTC (permalink / raw)
To: Adayadil Thomas; +Cc: netdev, Patrick McHardy
In-Reply-To: <fb7befa20910290843k12ab8ba6hbecb69bda8652ae0@mail.gmail.com>
Adayadil Thomas <adayadil.thomas@gmail.com> wrote:
>
> If two connections have same 5 tuple, src ip, dst ip, src port, dst
> port, protocol(tcp/udp)
> but on different vlans (different vlan id), does the conntrack separate these ?
Probably not. Patrick, can you confirm this?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 1/3] net: TCP thin-stream detection
From: apetlund @ 2009-10-30 15:23 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Arnd Hannemann, Andreas Petlund, William Allen Simpson,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
shemminger@vyatta.com, davem@davemloft.net, Christian Samsel
> On Thu, 29 Oct 2009, Arnd Hannemann wrote:
>
>> 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?
>
> I even gave some advise recently to one guy how to polish up the early
retransmit implementation of his. ...However, I think we haven't heard
from that since then... I added him as CC if he happens to have it
already
> done.
>
> It is actually so that patches 1+3 implement sort of an early
retransmit,
> just slightly more aggressive of it than what is given in ID but I find
the difference in the aggressiveness rather insignificant. ...Whereas
the
> RTO stuff is more questionable.
>
I share the opinion that the linear timeouts should be limited, and back
off exponentially after the limit, as Eric suggested. I believe this will
be a sufficient safety-valve for the black-hole scenario, although I would
like to run some tests.
As I wrote to Arnd, there are many similarities with the EFR approach and
what our patch does. The largest difference is that the thin-stream
patterns are identified as an indication of time dependent/interactive
apps. This is the reason why the proposed patch does not try to keep an
inflated cwnd open, but only focuses on the cases of few packets in
flight. The target is time-dependent/interactive applications, and as such
we don't want a generally enabled mechanism, but want to give the option
of enabling it only in the cases where they are most needed (in contrast
to a generally enabled "automagically" triggered EFR).
Below is a link to a table presenting some of the applications that we
have traced and analysed the packet interarrival times of:
http://folk.uio.no/apetlund/lktmp/thin_apps_table.pdf
We were surprised to see how many cases of "thin-stream" traffic patterns
were indicative of time-dependent/interactive apps.
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH 1/3] net: TCP thin-stream detection
From: Arnd Hannemann @ 2009-10-30 15:24 UTC (permalink / raw)
To: apetlund@simula.no
Cc: William Allen Simpson, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, shemminger@vyatta.com,
ilpo.jarvinen@helsinki.fi, davem@davemloft.net
In-Reply-To: <f7b4caf3a5b5c906cb8d508239455322.squirrel@webmail.uio.no>
apetlund@simula.no schrieb:
> 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.
Both mechanism prevent retransmission timeouts, thereby reducing latency.
Who cares, that they were motivated by performance?
I agree, that you are more aggressive, and that your scheme may have
latency advantages, at least for the Limited Transmit case. And there are
probably good reasons for your proposal. But I really think you should
bring your proposal up in IETF TCPM WG. I have the feeling that there are
a lot of corner cases we didn't think of.
One example: Consider standard NewReno non-SACK enabled flow:
For some reasons two data packets get reordered.
The TCP sender will produce a dupACK and an ACK.
The dupACK will trigger (because of your logic) a spurious retransmit.
The spurious retransmit will trigger a dupACK.
This dupACK will again trigger a spurious retransmit.
And this game will continue, unless a packet is dropped by coincidence.
P.S.: The Early-Rexmit ID has not been implemented in Linux,
because our student who was working on that is busy with something
else...
Best regards,
Arnd Hannemann
^ permalink raw reply
* Re: [PATCH 2/3] net: TCP thin linear timeouts
From: apetlund @ 2009-10-30 15:27 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Andreas Petlund, Eric Dumazet, Arnd Hannemann, Netdev, LKML,
shemminger, David Miller
> On Thu, 29 Oct 2009, apetlund@simula.no wrote:
>
>> > Andreas Petlund a écrit :
>> >
>> >> The removal of exponential backoff on a general basis has been
investigated and discussed already, for instance here:
>> >> http://ccr.sigcomm.org/online/?q=node/416
>> >> Such steps are, however considered drastic, and I agree that caution
>> must be made to thoroughly investigate the effects of such changes. The
changes introduced by the proposed patches, however, are not
>> default
>> >> behaviour, but an option for applications that suffer from the
thin-stream TCP increased retransmission latencies. They will, as
>> such,
>> not affect all streams. In addition, the changes will only be active for
>> >> streams which are perpetually thin or in the early phase of
expanding
>> their cwnd. Also, experiments performed on congested bottlenecks with
tail-drop queues show very little (if any at all) effect on goodput for
the modified scenario compared to a scenario with unmodified TCP
streams.
>> >> Graphs both for latency-results and fairness tests can be found
here:
>> http://folk.uio.no/apetlund/lktmp/
>> >
>> > There should be a limit to linear timeouts, to say ... no more than 6
>> retransmits
>> > (eventually tunable), then switch to exponential backoff. Maybe your
>> patch
>> > already implement such heuristic ?
>> The limitation you suggest to the linear timeouts makes very good
sense.
>> Our experiments performed on the Internet indicate that it is extremely
rare that more than 6 retransmissions are needed to recover. It is not
included in the current patch, so I will include this in the next
iteration.
>
> I've heard that BSD would use linear for first three and then
exponential
> but this is based on some gossip (which could well turn out to be a
myth)
> rather than checking it out myself. But if it is true, it certainly
hasn't
> been that devastating.
>> > True link collapses do happen, it would be good if not all streams
>> wakeup
>> > in the same
>> > second and make recovery very slow.
>> >
>> Each stream will have its own schedule for wakeup, so such events will
still be subject to coincidence. The timer granularity of the TCP
wakeup
>> timer will also influence how many streams will wake at the same time. The
>> experiments we have performed on severely congested bottlenecks (link
above) indicate that the modifications will not create a large negative
effect. In fact, when goodput is drastically reduced due to severe
overload, regular TCP and the LT and dupACK modifications seem to
perform
>> nearly identically. Other scenarios may exist where different effects can
>> be observed, and I am open to suggestions for further testing.
>
> Could you point out where exactly where the goodput results? ...I only
seem to find latency results which is not exactly the same. I don't
except
> some that is in order of what Nagle talks (32kbps -> 40bps irc) but
10-50%
> goodput reduction over a relatively short period of time (until RTTs top
RTOs once again preventing spurious RTOs and thus also segment
duplication
> due to retransmissions ceases).
The plot can be found here:
http://folk.uio.no/apetlund/lktmp/n-vs-n-fairness.pdf
I'm sorry that I didn't explain at once, as the parameters and setup is
not obvious. The boxplot shows aggregate throughput of all the unmodified,
greedy TCP New Reno streams when competing with thin streams using TCP New
Reno, linear timeouts, modified dupACK, RDB (which is not included this
patch set) and the combination of all the modifications. The streams
compete for a 1Mbps bottleneck that use tc with a tail-dropping queue to
limit bandwidth and netem to create loss and delay.
The RTT for the test is 100ms and the packet interarrival time for the
thin streams are 85ms.
> Were these results obtained with Linux, and if so what was FRTO set to?
The results are from our Linux implementation of the mechanisms. FRTO was
disabled and Nagle was disabled for all test sets.
>> > Thats too easy to accept possibly dangerous features with the excuse
>> of
>> saying
>> > "It wont be used very much", because you cannot predict the future.
>> I agree that it is no argument to say that it won't be used much; indeed,
>> my hope is that it will be used much. However, our experiments indicate no
>> negative effects while showing a large improvement on retransmission
latency for the scenario in question. I therefore think that the option
for such an improvement should be made available for time-dependent
thin-stream applications.
>
> Everyone can right away tell that most RTOs are not due to extreme
congestion, so some linear back off seems sensible when dupACK feedback
is lacking for some reason. Of course it is a tradeoff as there's that
chance for getting 1/(n+1) goodput only (where n is the number of linear
steps) step if RTOs were spurious (and without FRTO even more
unnecessary
> retransmission will be triggered so in fact even could be slightly worse
in theory). But that to happen in the first place requires of course
this
> RTT > RTO situation which is hard to see to be a persisting state.
Actually, we have found the low number of packets in flight to be a
persisting state in a large amount of applications that are interactive or
time-dependent. Some examples can be found in the table linked to below:
http://folk.uio.no/apetlund/lktmp/thin_apps_table.pdf
It seems that human interaction, sensor networks, and several other
scenarios that are not inherently greedy will produce a steady trickle of
data segments that fall into the "thin-stream" category and stays there.
Regards,
Andreas
^ permalink raw reply
* Re: Connection tracking and vlan
From: Eric Dumazet @ 2009-10-30 15:31 UTC (permalink / raw)
To: Herbert Xu; +Cc: Adayadil Thomas, netdev, Patrick McHardy
In-Reply-To: <20091030152054.GA7936@gondor.apana.org.au>
Herbert Xu a écrit :
> Adayadil Thomas <adayadil.thomas@gmail.com> wrote:
>> If two connections have same 5 tuple, src ip, dst ip, src port, dst
>> port, protocol(tcp/udp)
>> but on different vlans (different vlan id), does the conntrack separate these ?
>
> Probably not. Patrick, can you confirm this?
>
Very strange, this question about vlan looks like discussion we had
yesterday (or the day before...) about interfaces (versus packet defragmentation)
"IP conntracking" is about IP, and [V]LAN doesnt matter at all at this protocol level.
Same thing if you have two interfaces, eth0 & eth1 : IP conntrack tuples dont
include interface name/index
^ permalink raw reply
* Re: [PATCH] sky2: set carrier off in probe
From: Stephen Hemminger @ 2009-10-30 15:34 UTC (permalink / raw)
To: David Miller; +Cc: bphilips, netdev
In-Reply-To: <20091029.213816.35637194.davem@davemloft.net>
Why not fix the problem in a generic way?
---
Subject: ethtool: link is only up if device is running
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/core/ethtool.c 2009-10-30 08:32:52.584728610 -0700
+++ b/net/core/ethtool.c 2009-10-30 08:33:31.806667877 -0700
@@ -27,7 +27,7 @@
u32 ethtool_op_get_link(struct net_device *dev)
{
- return netif_carrier_ok(dev) ? 1 : 0;
+ return netif_running(dev) && netif_carrier_ok(dev);
}
u32 ethtool_op_get_rx_csum(struct net_device *dev)
^ permalink raw reply
* Re: Connection tracking and vlan
From: Herbert Xu @ 2009-10-30 15:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Adayadil Thomas, netdev, Patrick McHardy
In-Reply-To: <4AEB06E6.6020206@gmail.com>
On Fri, Oct 30, 2009 at 04:31:50PM +0100, Eric Dumazet wrote:
>
> Same thing if you have two interfaces, eth0 & eth1 : IP conntrack tuples dont
> include interface name/index
Indeed, but imagine what happens when eth0 is the LAN and eth1 is
the wild wild Internet. Do you really want their packets to mix?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: How to use gretap with bridge?
From: Herbert Xu @ 2009-10-30 15:51 UTC (permalink / raw)
To: Neulinger, Nathan, David S. Miller; +Cc: shemminger, netdev
In-Reply-To: <846C5B546E47494CBBD796CA8CA1617EA3B43C@MST-VMAIL1.srv.mst.edu>
Neulinger, Nathan <nneul@mst.edu> wrote:
>
> The above change fixes it for me, but I'm no expert on this chunk of
> code. (Perhaps it it shouldn't set dev_addr at all?)
OK, it was a stupid mistake on my part. I added a netdev ops
struct for tap but didn't actually use it! Please let us know
whether this patch fixes the problem.
gre: Fix dev_addr clobbering for gretap
Nathan Neulinger noticed that gretap devices get their MAC address
from the local IP address, which results in invalid MAC addresses
half of the time.
This is because gretap is still using the tunnel netdev ops rather
than the correct tap netdev ops struct.
This patch also fixes changelink to not clobber the MAC address
for the gretap case.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 41ada99..1433338 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1464,7 +1464,7 @@ static void ipgre_tap_setup(struct net_device *dev)
ether_setup(dev);
- dev->netdev_ops = &ipgre_netdev_ops;
+ dev->netdev_ops = &ipgre_tap_netdev_ops;
dev->destructor = free_netdev;
dev->iflink = 0;
@@ -1525,25 +1525,29 @@ static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[],
if (t->dev != dev)
return -EEXIST;
} else {
- unsigned nflags = 0;
-
t = nt;
- if (ipv4_is_multicast(p.iph.daddr))
- nflags = IFF_BROADCAST;
- else if (p.iph.daddr)
- nflags = IFF_POINTOPOINT;
+ if (dev->type != ARPHRD_ETHER) {
+ unsigned nflags = 0;
- if ((dev->flags ^ nflags) &
- (IFF_POINTOPOINT | IFF_BROADCAST))
- return -EINVAL;
+ if (ipv4_is_multicast(p.iph.daddr))
+ nflags = IFF_BROADCAST;
+ else if (p.iph.daddr)
+ nflags = IFF_POINTOPOINT;
+
+ if ((dev->flags ^ nflags) &
+ (IFF_POINTOPOINT | IFF_BROADCAST))
+ return -EINVAL;
+ }
ipgre_tunnel_unlink(ign, t);
t->parms.iph.saddr = p.iph.saddr;
t->parms.iph.daddr = p.iph.daddr;
t->parms.i_key = p.i_key;
- memcpy(dev->dev_addr, &p.iph.saddr, 4);
- memcpy(dev->broadcast, &p.iph.daddr, 4);
+ if (dev->type != ARPHRD_ETHER) {
+ memcpy(dev->dev_addr, &p.iph.saddr, 4);
+ memcpy(dev->broadcast, &p.iph.daddr, 4);
+ }
ipgre_tunnel_link(ign, t);
netdev_state_change(dev);
}
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox