* Re: [PATCH net-next-2.6] be2net: get rid of be_get_stats()
From: David Miller @ 2010-08-24 19:22 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, somnathk, ajitk
In-Reply-To: <1282663102.2477.253.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 24 Aug 2010 17:18:22 +0200
> drivers can avoid implementing ndo_get_stats method if using netdevice
> stats structure.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ethoc: get rid of ethoc_stats()
From: David Miller @ 2010-08-24 19:23 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1282663331.2477.255.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 24 Aug 2010 17:22:11 +0200
> drivers can avoid implementing ndo_get_stats method if using netdevice
> stats structure.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] bcm63xx_enet: use netdev stats
From: David Miller @ 2010-08-24 19:24 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, mbizon, ffainelli
In-Reply-To: <1282665195.2477.261.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 24 Aug 2010 17:53:15 +0200
> Use integrated net_device stats instead of a private one
>
> Get rid of bcm_enet_get_stats()
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* [PATCH net-next] include/linux/if_ether.h: Remove unused #define MAC_FMT
From: Joe Perches @ 2010-08-24 19:30 UTC (permalink / raw)
To: netdev; +Cc: LKML
Last use was removed, so remove the #define.
Signed-off-by: Joe Perches <joe@perches.com>
---
include/linux/if_ether.h | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index bed7a46..f9c3df0 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -137,8 +137,6 @@ extern struct ctl_table ether_table[];
extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
-#define MAC_FMT "%02x:%02x:%02x:%02x:%02x:%02x"
-
#endif
#endif /* _LINUX_IF_ETHER_H */
^ permalink raw reply related
* [PATCH] phylib: Fix race between returning phydev and calling adjust_link
From: Anton Vorontsov @ 2010-08-24 19:34 UTC (permalink / raw)
To: David S. Miller; +Cc: Andy Fleming, netdev, linux-kernel
It is possible that phylib will call adjust_link before returning
from {,of_}phy_connect(), which may cause the following [very rare,
though] oops upon reopening the device:
Unable to handle kernel paging request for data at address 0x0000024c
Oops: Kernel access of bad area, sig: 11 [#1]
PREEMPT SMP NR_CPUS=2 LTT NESTING LEVEL : 0
P1021 RDB
Modules linked in:
NIP: c0345dac LR: c0345dac CTR: c0345d84
TASK = dffab6b0[30] 'events/0' THREAD: c0d24000 CPU: 0
[...]
NIP [c0345dac] adjust_link+0x28/0x19c
LR [c0345dac] adjust_link+0x28/0x19c
Call Trace:
[c0d25f00] [000045e1] 0x45e1 (unreliable)
[c0d25f30] [c036c158] phy_state_machine+0x3ac/0x554
[...]
Here is why. Drivers store phydev in their private structures, e.g.
gianfar driver:
static int init_phy(struct net_device *dev)
{
...
priv->phydev = of_phy_connect(...);
...
}
So that adjust_link could retrieve it back:
static void adjust_link(struct net_device *dev)
{
...
struct phy_device *phydev = priv->phydev;
...
}
If the device has been opened before, then phydev->state is set to
PHY_HALTED (or undefined if the driver didn't call phy_stop()).
Now, phy_connect starts the PHY state machine before returning phydev to
the driver:
phy_start_machine(phydev, NULL);
if (phydev->irq > 0)
phy_start_interrupts(phydev);
return phydev;
The time between 'phy_start_machine()' and 'return phydev' is undefined.
The start machine routine delays execution for 1 second, which is enough
for most cases. But under heavy load, or if you're unlucky, it is quite
possible that PHY state machine will execute before phy_connect()
returns, and so adjust_link callback will try to dereference phydev,
which is not yet ready.
To fix the issue, simply initialize the PHY's state to PHY_READY during
phy_attach(). This will ensure that phylib won't call adjust_link before
phy_start().
Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
---
drivers/net/phy/phy_device.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index c076119..16ddc77 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -466,6 +466,8 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
phydev->interface = interface;
+ phydev->state = PHY_READY;
+
/* Do initial configuration here, now that
* we have certain key parameters
* (dev_flags and interface) */
--
1.7.0.5
^ permalink raw reply related
* Re: [PATCH net-next-2.6] bna: fix stats handling
From: David Miller @ 2010-08-24 19:36 UTC (permalink / raw)
To: rmody; +Cc: eric.dumazet, netdev, amathur, ddutt, huangj
In-Reply-To: <E5313AF6F2BFD14293E5FD0F94750F86A4AE225D69@HQ1-EXCH01.corp.brocade.com>
From: Rasesh Mody <rmody@brocade.com>
Date: Tue, 24 Aug 2010 11:49:45 -0700
> We tried applying the patch to the source, it looks corrupted. Can
> you please resubmit the patch?
His patch applies absolutely cleanly to net-next-2.6
I had to make fixes to your driver, as I mentioned when I checked
in your patch yesterday, so you absolutely must test against
what I added to net-next-2.6 and not what you have locally in
your trees.
^ permalink raw reply
* Re: RFC: MTU for serving NFS on Infiniband
From: Marc Aurele La France @ 2010-08-24 19:49 UTC (permalink / raw)
To: Ben Hutchings
Cc: Stephen Hemminger, linux-kernel, netdev, David S. Miller,
Alexey Kuznetsov, Pekka Savola (ipv6), James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <1282672647.2302.15.camel@achroite.uk.solarflarecom.com>
On Tue, 24 Aug 2010, Ben Hutchings wrote:
> On Tue, 2010-08-24 at 09:14 -0600, Marc Aurele La France wrote:
>> On Mon, 23 Aug 2010, Stephen Hemminger wrote:
>>> On Mon, 23 Aug 2010 08:44:37 -0600 (MDT)
>>> Marc Aurele La France <tsi@ualberta.ca> wrote:
>>>> In regrouping for my next tack at this, I noticed that all stack traces go
>>>> through ip_append_data(). This would be ipv6_append_data() in the IPv6 case.
>>>> A _very_ rough draft that would have ip_append_data() temporarily drop down
>>>> to a smaller fake MTU follows ...
>>> Why doesn't NFS generate page size fragments? Does Infiniband or your
>>> device not support this? Any thing that requires higher order allocation
>>> is going to unstable under load. Let's fix the cause not the apply bandaid
>>> solution to the symptom.
>> From what I can tell, IP fragmentation is done centrally.
> [...]
> Stephen and I are not talking about IP fragmentation, but about the
> ability to append 'fragments' to an skb rather than putting the entire
> packet payload in a linear buffer. See
> <http://vger.kernel.org/~davem/skb_data.html>.
Any payload has to either fit in the MTU, or has to be broken up into
MTU-sized (or less) fragments, come hell or high water. That this is done
centrally is a good thing. It is the "(or less)" part that I am working
towards here.
Marc.
+----------------------------------+----------------------------------+
| Marc Aurele La France | work: 1-780-492-9310 |
| Academic Information and | fax: 1-780-492-1729 |
| Communications Technologies | email: tsi@ualberta.ca |
| 352 General Services Building +----------------------------------+
| University of Alberta | |
| Edmonton, Alberta | Standard disclaimers apply |
| T6G 2H1 | |
| CANADA | |
+----------------------------------+----------------------------------+
^ permalink raw reply
* Re: RFC: MTU for serving NFS on Infiniband
From: Eric Dumazet @ 2010-08-24 20:09 UTC (permalink / raw)
To: Marc Aurele La France
Cc: Ben Hutchings, Stephen Hemminger, linux-kernel, netdev,
David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <alpine.WNT.2.00.1008241338470.1132@cluij.ucs.ualberta.ca>
Le mardi 24 août 2010 à 13:49 -0600, Marc Aurele La France a écrit :
>
> Any payload has to either fit in the MTU, or has to be broken up into
> MTU-sized (or less) fragments, come hell or high water. That this is done
> centrally is a good thing. It is the "(or less)" part that I am working
> towards here.
>
Could you post a full stack trace, to help me understand the path from
NFS to ip_append_data ?
I suspect this is UDP transport ?
This reminds me a patch I wrote for IPV6 : We were allocating a huge
(MTU sized) buffer, just to fill few bytes in it...
commit 72e09ad107e78d69ff4d3b97a69f0aad2b77280f
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat Jun 5 03:03:30 2010 -0700
ipv6: avoid high order allocations
With mtu=9000, mld_newpack() use order-2 GFP_ATOMIC allocations, that
are very unreliable, on machines where PAGE_SIZE=4K
Limit allocated skbs to be at most one page. (order-0 allocations)
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: RFC: MTU for serving NFS on Infiniband
From: Marc Aurele La France @ 2010-08-24 20:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, Stephen Hemminger, linux-kernel, netdev,
David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
In-Reply-To: <1282680572.2467.78.camel@edumazet-laptop>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3433 bytes --]
On Tue, 24 Aug 2010, Eric Dumazet wrote:
> Le mardi 24 août 2010 à 13:49 -0600, Marc Aurele La France a écrit :
>> Any payload has to either fit in the MTU, or has to be broken up into
>> MTU-sized (or less) fragments, come hell or high water. That this is done
>> centrally is a good thing. It is the "(or less)" part that I am working
>> towards here.
> Could you post a full stack trace, to help me understand the path from
> NFS to ip_append_data ?
[<ffffffff810a5abe>] __alloc_pages_nodemask+0x617/0x692
[<ffffffff81061688>] ? mark_held_locks+0x49/0x64
[<ffffffff810d018b>] kmalloc_large_node+0x61/0x9e
[<ffffffff810d3050>] __kmalloc_node_track_caller+0x32/0x159
[<ffffffff812612da>] ? sock_alloc_send_pskb+0xc9/0x2ea
[<ffffffff81265cc6>] __alloc_skb+0x74/0x163
[<ffffffff812612da>] sock_alloc_send_pskb+0xc9/0x2ea
[<ffffffff81061688>] ? mark_held_locks+0x49/0x64
[<ffffffff81261510>] sock_alloc_send_skb+0x15/0x17
[<ffffffff81299317>] ip_append_data+0x500/0x9d0
[<ffffffff8103feae>] ? local_bh_enable+0xb7/0xbd
[<ffffffff8129a804>] ? ip_generic_getfrag+0x0/0x92
[<ffffffff81292bcd>] ? ip_route_output_flow+0x82/0x1f9
[<ffffffff812b8990>] udp_sendmsg+0x4ec/0x60c
[<ffffffff812bf2ac>] inet_sendmsg+0x4b/0x58
[<ffffffff8125dd89>] sock_sendmsg+0xd9/0xfa
[<ffffffff81063fb0>] ? __lock_acquire+0x787/0x7f5
[<ffffffff81063fb0>] ? __lock_acquire+0x787/0x7f5
[<ffffffff8125fcf5>] kernel_sendmsg+0x37/0x43
[<ffffffffa0267cd2>] xs_send_kvec+0x88/0x93 [sunrpc]
[<ffffffff812f08dc>] ? _raw_spin_unlock_irqrestore+0x44/0x4c
[<ffffffffa0267d5c>] xs_sendpages+0x7f/0x1be [sunrpc]
[<ffffffffa026952f>] xs_udp_send_request+0x5b/0x103 [sunrpc]
[<ffffffffa0266c0a>] xprt_transmit+0x11f/0x1f5 [sunrpc]
[<ffffffffa02ea140>] ? nfs3_xdr_writeargs+0x0/0x82 [nfs]
[<ffffffffa02648b9>] call_transmit+0x218/0x25e [sunrpc]
[<ffffffffa026aced>] __rpc_execute+0x9b/0x288 [sunrpc]
[<ffffffffa026aeef>] rpc_async_schedule+0x15/0x17 [sunrpc]
[<ffffffff81051137>] worker_thread+0x1ed/0x2e6
[<ffffffff810510e1>] ? worker_thread+0x197/0x2e6
[<ffffffffa026aeda>] ? rpc_async_schedule+0x0/0x17 [sunrpc]
[<ffffffff8105450f>] ? autoremove_wake_function+0x0/0x3d
[<ffffffff81050f4a>] ? worker_thread+0x0/0x2e6
[<ffffffff810541b2>] kthread+0x82/0x8a
[<ffffffff81002f14>] kernel_thread_helper+0x4/0x10
[<ffffffff81030d20>] ? finish_task_switch+0x0/0xd6
[<ffffffff81002f10>] ? kernel_thread_helper+0x0/0x10
There are many other variations as well.
> I suspect this is UDP transport ?
Yes.
> This reminds me a patch I wrote for IPV6 : We were allocating a huge
> (MTU sized) buffer, just to fill few bytes in it...
Humm. Interesting. Thanks for the pointer.
Marc.
+----------------------------------+----------------------------------+
| Marc Aurele La France | work: 1-780-492-9310 |
| Academic Information and | fax: 1-780-492-1729 |
| Communications Technologies | email: tsi@ualberta.ca |
| 352 General Services Building +----------------------------------+
| University of Alberta | |
| Edmonton, Alberta | Standard disclaimers apply |
| T6G 2H1 | |
| CANADA | |
+----------------------------------+----------------------------------+
^ permalink raw reply
* Re: [PATCH] TCP_FAILFAST: a new socket option to timeout/abort a connection quicker
From: Jerry Chu @ 2010-08-24 20:47 UTC (permalink / raw)
To: Eric Dumazet; +Cc: ilpo.jarvinen, davem, netdev
In-Reply-To: <1282632262.2378.1681.camel@edumazet-laptop>
On Mon, Aug 23, 2010 at 11:44 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 23 août 2010 à 23:20 -0700, H.K. Jerry Chu a écrit :
>> From: Jerry Chu <hkchu@google.com>
>>
>> This is a TCP level socket option that takes an unsigned int to specify
>> how long in ms TCP should resend a lost data packet before giving up
>> and returning ETIMEDOUT. The normal TCP retry/abort timeout limit still
>> applies. In other words this option is only meant for those applications
>> that need to "fail faster" than the default TCP timeout. The latter
>> may take upto 20 minutes in a normal WAN environment.
>>
>> The option is disabled (by default) when set to 0. Also it does not
>> apply during the connection establishment phase.
>>
>> Signed-off-by: H.K. Jerry Chu <hkchu@google.com>
>
> TCP_FAILFAST might be misleading. It reads as a boolean option, while
> its an option to cap the timeout, with a time unit, instead of the usual
> "number of retransmits".
I'm open to better names. Perhaps it can be combined with TCP_UTO
mentioned in subsequent reply?
>
> Its also funny you dont ask for a default value, given by a sysctl
> tunable ;)
This socket option takes time unit directly. The other sysctls
use # of retries and max_rto. (Not sure if that's what you asked.)
>
>
>
>
^ permalink raw reply
* RE: [PATCH net-next-2.6] bna: fix stats handling
From: Rasesh Mody @ 2010-08-24 20:48 UTC (permalink / raw)
To: David Miller, eric.dumazet@gmail.com
Cc: netdev@vger.kernel.org, Akshay Mathur, Debashis Dutt, Jing Huang
In-Reply-To: <20100824.123645.39174077.davem@davemloft.net>
Hi Eric, David,
It was a mistake, I think it was a false alarm due to an email client issue. The patch was applied to net-next-2.6 and it went through cleanly. We are testing the patch and the fixes. We will let you know the results soon.
>diff --git a/drivers/net/bna/bnad.c b/drivers/net/bna/bnad.c index 491d148..a935b96 100644
>--- a/drivers/net/bna/bnad.c
>+++ b/drivers/net/bna/bnad.c
>@@ -1964,25 +1964,24 @@ bnad_enable_default_bcast(struct bnad *bnad)
>
> /* Statistics utilities */
> void
>-bnad_netdev_qstats_fill(struct bnad *bnad)
>+bnad_netdev_qstats_fill(struct bnad *bnad, struct rtnl_link_stats64
>+*stats)
It looks like the email client split the line and also added a + sign, giving an illusion of insertion which was not actually there.
Thanks,
Rasesh
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, August 24, 2010 12:37 PM
To: Rasesh Mody
Cc: eric.dumazet@gmail.com; netdev@vger.kernel.org; Akshay Mathur; Debashis Dutt; Jing Huang
Subject: Re: [PATCH net-next-2.6] bna: fix stats handling
From: Rasesh Mody <rmody@brocade.com>
Date: Tue, 24 Aug 2010 11:49:45 -0700
> We tried applying the patch to the source, it looks corrupted. Can
> you please resubmit the patch?
His patch applies absolutely cleanly to net-next-2.6
I had to make fixes to your driver, as I mentioned when I checked
in your patch yesterday, so you absolutely must test against
what I added to net-next-2.6 and not what you have locally in
your trees.
^ permalink raw reply
* [PATCH] octeon: depends on NETDEVICES
From: Randy Dunlap @ 2010-08-24 21:09 UTC (permalink / raw)
To: devel, netdev; +Cc: gregkh, support, Arnaud Lacombe
From: Randy Dunlap <randy.dunlap@oracle.com>
OCTEON_ETHERNET should depend on NETDEVICES.
Fixes this kconfig warning:
warning: (NET_DSA && NET && EXPERIMENTAL && NETDEVICES && !S390 || ...
|| OCTEON_ETHERNET && STAGING && !STAGING_EXCLUDE_BUILD && CPU_CAVIUM_OCTEON) selects PHYLIB which has unmet direct dependencies (!S390 && NETDEVICES)
Reported-by: Arnaud Lacombe <lacombar@gmail.com>
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Cc: support@caviumnetworks.com
---
drivers/staging/octeon/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20100823.orig/drivers/staging/octeon/Kconfig
+++ linux-next-20100823/drivers/staging/octeon/Kconfig
@@ -1,6 +1,6 @@
config OCTEON_ETHERNET
tristate "Cavium Networks Octeon Ethernet support"
- depends on CPU_CAVIUM_OCTEON
+ depends on CPU_CAVIUM_OCTEON && NETDEVICES
select PHYLIB
select MDIO_OCTEON
help
^ permalink raw reply
* [PATCH net-next-2.6] net: ip_append_data() optim
From: Eric Dumazet @ 2010-08-24 21:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Compiler is not smart enough to avoid a conditional branch.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/ip_output.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 6d2753c..e427620 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -837,10 +837,9 @@ int ip_append_data(struct sock *sk,
inet->cork.length = 0;
sk->sk_sndmsg_page = NULL;
sk->sk_sndmsg_off = 0;
- if ((exthdrlen = rt->dst.header_len) != 0) {
- length += exthdrlen;
- transhdrlen += exthdrlen;
- }
+ exthdrlen = rt->dst.header_len;
+ length += exthdrlen;
+ transhdrlen += exthdrlen;
} else {
rt = (struct rtable *)inet->cork.dst;
if (inet->cork.flags & IPCORK_OPT)
^ permalink raw reply related
* Re: [PATCH net-next-2.6] net: ip_append_data() optim
From: David Miller @ 2010-08-24 21:45 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1282685687.2467.175.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 24 Aug 2010 23:34:47 +0200
> Compiler is not smart enough to avoid a conditional branch.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] phylib: Fix race between returning phydev and calling adjust_link
From: David Miller @ 2010-08-24 21:46 UTC (permalink / raw)
To: avorontsov; +Cc: afleming, netdev, linux-kernel
In-Reply-To: <20100824193412.GA1887@oksana.dev.rtsoft.ru>
From: Anton Vorontsov <avorontsov@mvista.com>
Date: Tue, 24 Aug 2010 23:34:12 +0400
> It is possible that phylib will call adjust_link before returning
> from {,of_}phy_connect(), which may cause the following [very rare,
> though] oops upon reopening the device:
...
> To fix the issue, simply initialize the PHY's state to PHY_READY during
> phy_attach(). This will ensure that phylib won't call adjust_link before
> phy_start().
>
> Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Applied, thanks Anton.
^ permalink raw reply
* Re: [PATCH net-next] include/linux/if_ether.h: Remove unused #define MAC_FMT
From: David Miller @ 2010-08-24 21:47 UTC (permalink / raw)
To: joe; +Cc: netdev, linux-kernel
In-Reply-To: <1282678211.25208.1.camel@Joe-Laptop>
From: Joe Perches <joe@perches.com>
Date: Tue, 24 Aug 2010 12:30:11 -0700
> Last use was removed, so remove the #define.
>
> Signed-off-by: Joe Perches <joe@perches.com>
yippie :) Applied, thanks Joe.
^ permalink raw reply
* Re: [PATCH] tc: add meta match on receive hash
From: David Miller @ 2010-08-24 21:48 UTC (permalink / raw)
To: shemminger; +Cc: xiaosuo, hadi, tgraf, netdev
In-Reply-To: <20100824111440.37eebae1@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 24 Aug 2010 11:14:40 -0700
> Trivial extension to existing meta data match rules to allow
> matching on skb receive hash value.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied, thanks Stephen.
^ permalink raw reply
* Re: [patch 1/4] pxa168_eth: remove unneeded null check
From: David Miller @ 2010-08-24 21:50 UTC (permalink / raw)
To: error27; +Cc: ssanap, netdev, kernel-janitors
In-Reply-To: <20100824165150.GH29330@bicker>
From: Dan Carpenter <error27@gmail.com>
Date: Tue, 24 Aug 2010 18:52:46 +0200
> "pep->pd" isn't checked consistently in this function. For example it's
> dereferenced unconditionally on the next line after the end of the if
> condition. This function is only called from pxa168_eth_probe() and
> pep->pd is always non-NULL so I removed the check.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Applied.
^ permalink raw reply
* Re: [patch 2/4] pxa168_eth: fix error handling in prope
From: David Miller @ 2010-08-24 21:50 UTC (permalink / raw)
To: error27; +Cc: ssanap, netdev, kernel-janitors
In-Reply-To: <20100824165333.GI29330@bicker>
From: Dan Carpenter <error27@gmail.com>
Date: Tue, 24 Aug 2010 18:53:33 +0200
> A couple issues here:
> * Some resources weren't released.
> * If alloc_etherdev() failed it would have caused a NULL dereference
> because "pep" would be null when we checked "if (pep->clk)".
> * Also it's better to propagate the error codes from mdiobus_register()
> instead of just returning -ENOMEM.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Applied.
^ permalink raw reply
* Re: [patch 3/4] pxa168_eth: update call to phy_mii_ioctl()
From: David Miller @ 2010-08-24 21:50 UTC (permalink / raw)
To: error27; +Cc: ssanap, netdev, kernel-janitors
In-Reply-To: <20100824165420.GJ29330@bicker>
From: Dan Carpenter <error27@gmail.com>
Date: Tue, 24 Aug 2010 18:54:20 +0200
> The phy_mii_ioctl() function changed recently. It now takes a struct
> ifreq pointer directly.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Applied.
^ permalink raw reply
* Re: [patch 4/4] pxa168_eth: silence gcc warnings
From: David Miller @ 2010-08-24 21:51 UTC (permalink / raw)
To: error27; +Cc: ssanap, netdev, kernel-janitors
In-Reply-To: <20100824165505.GK29330@bicker>
From: Dan Carpenter <error27@gmail.com>
Date: Tue, 24 Aug 2010 18:55:05 +0200
> Casting "pep->tx_desc_dma" to to a struct tx_desc pointer makes gcc
> complain:
>
> drivers/net/pxa168_eth.c:657: warning:
> cast to pointer from integer of different size
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] TCP_FAILFAST: a new socket option to timeout/abort a connection quicker
From: Jerry Chu @ 2010-08-24 21:56 UTC (permalink / raw)
To: Arnd Hannemann; +Cc: Eric Dumazet, ilpo.jarvinen, davem, netdev
In-Reply-To: <4C737D15.5060400@nets.rwth-aachen.de>
On Tue, Aug 24, 2010 at 1:04 AM, Arnd Hannemann
<hannemann@nets.rwth-aachen.de> wrote:
> Am 24.08.2010 08:44, schrieb Eric Dumazet:
>> Le lundi 23 août 2010 à 23:20 -0700, H.K. Jerry Chu a écrit :
>>> From: Jerry Chu <hkchu@google.com>
>>>
>>> This is a TCP level socket option that takes an unsigned int to specify
>>> how long in ms TCP should resend a lost data packet before giving up
>>> and returning ETIMEDOUT. The normal TCP retry/abort timeout limit still
>>> applies. In other words this option is only meant for those applications
>>> that need to "fail faster" than the default TCP timeout. The latter
>>> may take upto 20 minutes in a normal WAN environment.
>>>
>>> The option is disabled (by default) when set to 0. Also it does not
>>> apply during the connection establishment phase.
>>>
>>> Signed-off-by: H.K. Jerry Chu <hkchu@google.com>
>>
>> TCP_FAILFAST might be misleading. It reads as a boolean option, while
>> its an option to cap the timeout, with a time unit, instead of the usual
>> "number of retransmits".
>
> Why not call it TCP_USERTIMEOUT?
Sure, except that it was designed to shorten the system default user timeout,
not lengthen it. (But perhaps it can be combined with TCP_UTO?)
The current default user timeout of 13-20minutes in Linux may be adequate for
some apps but too long for many others. A per connection socket option solves
this problem.
> Later you can also send it via the TCP user timeout option... (RFC5482)
> Hmm... is the ms granularity really needed? Does it make sense to abort
> a connection below a second?
Yes I thought about that too, but decided it's better to allow the
flexibility of sub-
sec level timeout for possible future usage in HPC type of applications, rather
than to regret later.
>
>> Its also funny you dont ask for a default value, given by a sysctl
>> tunable ;)
>
> Well retries1/2 would be the tunables, no?
The was my first thought, to allow tcp_retries2 to be reduced on a per
connection basis. But I also see a need to reduce TCP_RTO_MAX in
order to allow a reasonable # of retries, given a shorter timeout.
I saw a patch submitted a couple of months ago to allow tcp_retries2 to be
configured but haven't seen any forward progress on the patch. If people
think letting apps configure tcp_retries2 and TCP_RTO_MAX directly is
a better solution I'm for it too.
Thanks,
Jerry
>
> Best regards,
> Arnd
>
^ permalink raw reply
* Re: [PATCH 01/14] mlx4_en: Fixed incorrect unmapping on RX flow.
From: David Miller @ 2010-08-24 22:07 UTC (permalink / raw)
To: yevgenyp; +Cc: netdev, eugenia
In-Reply-To: <4C73CCE1.6050609@mellanox.co.il>
All 14 patches applied, but patches #12 and #13 had a lot of conflicts
because the LRO options were already deleted by the patch I applied
last week from Amerigo Wang:
commit f3c58aceaa3f237ba43735805f4677950327b8ee
Author: Amerigo Wang <amwang@redhat.com>
Date: Tue Aug 17 21:51:18 2010 +0000
mlx4: remove num_lro parameter
As suggested by David, this parameter can die, we can use ethtool
to turn LRO on/off. Compile tests only.
Signed-off-by: WANG Cong <amwang@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
I fixed up the conflicts but please generate your patches against
the correct tree in the future.
I also had to add the following build warning fix after applying your
patches, do you look at the build output at allwhen you make changes
to this driver?
--------------------
mlx4_en: Fix build warning in mlx4_en_create_rx_ring.
drivers/net/mlx4/en_rx.c: In function ‘mlx4_en_create_rx_ring’:
drivers/net/mlx4/en_rx.c:305: warning: label ‘err_map’ defined but not used
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/mlx4/en_rx.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mlx4/en_rx.c b/drivers/net/mlx4/en_rx.c
index cb7ff00..c4aad7f 100644
--- a/drivers/net/mlx4/en_rx.c
+++ b/drivers/net/mlx4/en_rx.c
@@ -302,8 +302,6 @@ int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
return 0;
-err_map:
- mlx4_en_unmap_buffer(&ring->wqres.buf);
err_hwq:
mlx4_free_hwq_res(mdev->dev, &ring->wqres, ring->buf_size);
err_ring:
--
1.7.2.2
^ permalink raw reply related
* Re: [PATCH] TCP_FAILFAST: a new socket option to timeout/abort a connection quicker
From: Jerry Chu @ 2010-08-24 22:13 UTC (permalink / raw)
To: Hagen Paul Pfeifer
Cc: Arnd Hannemann, Eric Dumazet, ilpo.jarvinen, davem, netdev
In-Reply-To: <20100824162844.GA7889@nuttenaction>
On Tue, Aug 24, 2010 at 9:28 AM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> * Arnd Hannemann | 2010-08-24 16:58:58 [+0200]:
>
>>Nice, so did you come up with a name for the socket option yet?
>
> +#define TCP_UTO 18 /* User Timeout Option */
>
> The patch is an early state and details as well as testing is a little bit
> costly.
>
>>Hmm, is there really a difference? If an application specifies
>>a wanted timeout e.g. with USER_TIMEOUT, CHANGEABLE will
>>become false and the value would be announced via ADV_UTO.
>>The connection could be aborted locally after that time passed,
>>regardless of what the remote site thinks the timeout should be.
>>
>>As I understand it U_LIMIT and L_LIMIT would only be there
>>for safety to disallow nonsensical values of USER_TIMEOUT.
>>
>>Did I miss something?
>
> Maybe not, aot sure. I must take a look at the patch from Jerry. I had no time
> until now.
According to RFC5482
"Decreasing the user timeouts allows busy servers to explicitly notify
their clients that they will maintain the connection state only for a
short time without connectivity."
So it looks like the user timeout can be used in either senario (shortening
or lengthening) and in both cases is a lower bound, i.e., the connection
should abort at or shortly after the specified user timeout.
In this case does it make sense to combine the two? Will your TCP_UTO
patch be ready anytime soon?
Again an alternative is to allow configuring tcp_retries2 and TCP_RTO_MAX
directly. I'm open to suggestion but we'd like to get something in sooner.
Thanks,
Jerry
>
> Hagen
>
^ permalink raw reply
* Using GPU to do packet forwarding
From: Stephen Hemminger @ 2010-08-24 22:15 UTC (permalink / raw)
To: David Miller, Eric Dumazet, netdev
Interesting paper:
http://www.ndsl.kaist.edu/~kyoungsoo/papers/packetshader.pdf
One section of general consideration is the expense of the current
skb scheme. At 10G, they measure 60% of the CPU is used doing skb
alloc/free; see paper for the alternative of using a huge
packet buffer. Also, they managed to shrink skb to 8 bytes!
^ 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