* [IPv6] Fix ICMPv6 redirect handling with target multicast address, try 3
From: Brian Haley @ 2007-10-03 14:44 UTC (permalink / raw)
To: David Miller, YOSHIFUJI Hideaki; +Cc: netdev@vger.kernel.org, David Stevens
In-Reply-To: <4702997D.6020208@hp.com>
[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]
When the ICMPv6 Target address is multicast, Linux processes the
redirect instead of dropping it. The problem is in this code in
ndisc_redirect_rcv():
if (ipv6_addr_equal(dest, target)) {
on_link = 1;
} else if (!(ipv6_addr_type(target) & IPV6_ADDR_LINKLOCAL)) {
ND_PRINTK2(KERN_WARNING
"ICMPv6 Redirect: target address is not
link-local.\n");
return;
}
This second check will succeed if the Target address is, for example,
FF02::1 because it has link-local scope. Instead, it should be checking
if it's a unicast link-local address, as stated in RFC 2461/4861 Section
8.1:
- The ICMP Target Address is either a link-local address (when
redirected to a router) or the same as the ICMP Destination
Address (when redirected to the on-link destination).
I know this doesn't explicitly say unicast link-local address, but it's
implied.
This bug is preventing Linux kernels from achieving IPv6 Logo Phase II
certification because of a recent error that was found in the TAHI test
suite - Neighbor Disovery suite test 206 (v6LC.2.3.6_G) had the
multicast address in the Destination field instead of Target field, so
we were passing the test. This won't be the case anymore.
The patch below fixes this problem, and also fixes ndisc_send_redirect()
to not send an invalid redirect with a multicast address in the Target
field. I re-ran the TAHI Neighbor Discovery section to make sure Linux
passes all 245 tests now.
-Brian
Signed-off-by: Brian Haley <brian.haley@hp.com>
Acked-by: David L Stevens <dlstevens@us.ibm.com>
[-- Attachment #2: ipv6.redirect.patch --]
[-- Type: text/x-patch, Size: 1051 bytes --]
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 74c4d8d..b761dbe 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1267,9 +1267,10 @@ static void ndisc_redirect_rcv(struct sk_buff *skb)
if (ipv6_addr_equal(dest, target)) {
on_link = 1;
- } else if (!(ipv6_addr_type(target) & IPV6_ADDR_LINKLOCAL)) {
+ } else if (ipv6_addr_type(target) !=
+ (IPV6_ADDR_UNICAST|IPV6_ADDR_LINKLOCAL)) {
ND_PRINTK2(KERN_WARNING
- "ICMPv6 Redirect: target address is not link-local.\n");
+ "ICMPv6 Redirect: target address is not link-local unicast.\n");
return;
}
@@ -1343,9 +1344,9 @@ void ndisc_send_redirect(struct sk_buff *skb, struct neighbour *neigh,
}
if (!ipv6_addr_equal(&ipv6_hdr(skb)->daddr, target) &&
- !(ipv6_addr_type(target) & IPV6_ADDR_LINKLOCAL)) {
+ ipv6_addr_type(target) != (IPV6_ADDR_UNICAST|IPV6_ADDR_LINKLOCAL)) {
ND_PRINTK2(KERN_WARNING
- "ICMPv6 Redirect: target address is not link-local.\n");
+ "ICMPv6 Redirect: target address is not link-local unicast.\n");
return;
}
^ permalink raw reply related
* Re: [PATCH net-2.6.24 0/3]: More TCP fixes
From: Ilpo Järvinen @ 2007-10-03 14:22 UTC (permalink / raw)
To: Cedric Le Goater; +Cc: David Miller, Netdev
In-Reply-To: <4703A1BB.1040006@free.fr>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1480 bytes --]
On Wed, 3 Oct 2007, Cedric Le Goater wrote:
> Ilpo Järvinen wrote:
> >
> > Ah, that's path 1) then... Since you seem to have enough time, I would say
> > that the path 1 is good as well and bugs unrelated to the fix will show up
> > there too...
>
> arg. yes. sorry for the confusion.
>
> > I should have stated it explicitly that with path 2 those 3 patches should
> > not be applied because the aim is not a fix but reproducal. Path 2 was
> > intentionally left without the potentional fix as then nice backtrace
> > informs when we can stop trying (which would hopefully occurred
> > pretty soon) :-). But lets discard that path 2...
>
> I have 2 spare nodes so i'll run both. 1) is on already without any issues
> i'm just compiling 2)
Thanks a lot. :-)
> I usually work on -mm, so what would be interesting for me is to have what you
> need in net-2.6.24 which is getting pulled in -mm by andrew. then, if
> you need an extra patch for verbosity, that's fine, i'll include it in
> my usual patchset.
Ah, I'm sorry about the subject and the extra work it caused, it was
meant for DaveM only, didn't realize at that time it would be
meaningful to you as well, thus couldn't warn you back then... Testing on
top of mm would be (/ have been) fine as well... From my point of view
both mm and net-2.6.24 are pretty much the same (I even verified that
those patches apply fine on top of rc8-mm2 since I thought that you might
want to use that one).
--
i.
^ permalink raw reply
* Re: Please pull 'upstream-davem' branch of wireless-2.6
From: John W. Linville @ 2007-10-03 14:10 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20071002.190156.41633562.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Tue, Oct 02, 2007 at 07:01:56PM -0700, David Miller wrote:
> From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> Date: Tue, 2 Oct 2007 21:25:52 -0400
>
> > git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git upstream-davem
>
> This doesn't pull cleanly.
>
> Probably you used a recently cloned Linus tree, pulled
> net-2.6.24 into that (and resolved the conflicts), and
> then put your patches in.
No, in fact I'm quite conscious of that. I follow a procedure
identical to what you outlined. I even leave my 'master-davem' branch
available as a reference, and create the initial 'upstream-davem'
branch as a checkout from it. :-)
As an experiment, I cloned your current tree (which has the patches
applied already, thanks!) and created a branch which backed-out the
patches from me you had already applied by hand. I then did a pull
from my tree, and the results were quite clean.
[linville]:> git checkout -b jwltest fc26d79bb258b5fdb3dee940bea12d6ef7c217c5
Switched to a new branch "jwltest"
[linville]:> git pull git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git upstream-davem
remote: Generating pack...
remote: Done counting 257 objects.
remote: Result has 199 objects.
remote: Deltifying 199 objects...
remote: 100% (199/199) done
Indexing 199 objects...
remote: Total 199 (delta 150), reused 143 (delta 115)
100% (199/199) done
Resolving 150 deltas...
100% (150/150) done
32 objects were added to complete this thin pack.
Removed drivers/net/wireless/zd1211rw/zd_util.c
Removed drivers/net/wireless/zd1211rw/zd_util.h
Merge made by recursive.
Documentation/networking/mac80211-injection.txt | 32 ++-
drivers/net/wireless/adm8211.c | 8 +-
drivers/net/wireless/b43/Kconfig | 12 +
drivers/net/wireless/b43/Makefile | 5 +-
drivers/net/wireless/b43/b43.h | 11 +-
drivers/net/wireless/b43/leds.c | 399 ++++++++++-------------
drivers/net/wireless/b43/leds.h | 63 ++--
drivers/net/wireless/b43/main.c | 205 ++++--------
drivers/net/wireless/b43/phy.c | 13 +-
drivers/net/wireless/b43/phy.h | 2 +-
drivers/net/wireless/b43/rfkill.c | 184 +++++++++++
drivers/net/wireless/b43/rfkill.h | 58 ++++
drivers/net/wireless/hostap/hostap.h | 2 +-
drivers/net/wireless/hostap/hostap_hw.c | 2 +-
drivers/net/wireless/hostap/hostap_main.c | 19 +-
drivers/net/wireless/iwlwifi/iwl3945-base.c | 4 -
drivers/net/wireless/iwlwifi/iwl4965-base.c | 4 -
drivers/net/wireless/p54common.c | 4 +-
drivers/net/wireless/p54pci.c | 4 +-
drivers/net/wireless/rt2x00/rt2x00.h | 2 +-
drivers/net/wireless/zd1211rw/Makefile | 2 +-
drivers/net/wireless/zd1211rw/zd_chip.c | 1 -
drivers/net/wireless/zd1211rw/zd_mac.c | 4 +-
drivers/net/wireless/zd1211rw/zd_usb.c | 1 -
drivers/net/wireless/zd1211rw/zd_util.c | 82 -----
drivers/net/wireless/zd1211rw/zd_util.h | 29 --
include/linux/rfkill.h | 24 ++
include/net/mac80211.h | 46 +++-
net/mac80211/cfg.c | 75 ++++-
net/mac80211/ieee80211.c | 189 +-----------
net/mac80211/ieee80211_i.h | 17 +-
net/mac80211/ieee80211_iface.c | 68 +----
net/mac80211/ieee80211_ioctl.c | 31 +-
net/mac80211/ieee80211_led.c | 67 +++-
net/mac80211/ieee80211_led.h | 6 +
net/mac80211/ieee80211_rate.c | 3 +-
net/mac80211/ieee80211_rate.h | 2 -
net/mac80211/ieee80211_sta.c | 7 +-
net/mac80211/key.c | 1 -
net/mac80211/rx.c | 122 +++-----
net/mac80211/sta_info.c | 13 +-
net/mac80211/tx.c | 211 ++++++------
net/mac80211/wme.c | 10 +-
net/rfkill/Kconfig | 7 +
net/rfkill/rfkill.c | 49 +++-
45 files changed, 1022 insertions(+), 1078 deletions(-)
create mode 100644 drivers/net/wireless/b43/rfkill.c
create mode 100644 drivers/net/wireless/b43/rfkill.h
delete mode 100644 drivers/net/wireless/zd1211rw/zd_util.c
delete mode 100644 drivers/net/wireless/zd1211rw/zd_util.h
So I'm not sure what happened for you. But I think it must have been
some other anomaly.
Anyway, I'm sorry it wasn't as smooth as usual. Thanks for the manual
patch work!
John
--
John W. Linville
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org
^ permalink raw reply
* Re: [PATCH net-2.6.24 0/3]: More TCP fixes
From: Cedric Le Goater @ 2007-10-03 14:05 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: David Miller, Netdev
In-Reply-To: <Pine.LNX.4.64.0710031554380.27745@kivilampi-30.cs.helsinki.fi>
Ilpo Järvinen wrote:
> On Wed, 3 Oct 2007, Cedric Le Goater wrote:
>
>> Ilpo Järvinen wrote:
>>> On Wed, 3 Oct 2007, Cedric Le Goater wrote:
>>>
>>>> I'm dropping the previous patches you sent me and switching to this patchset.
>>>> right ?
>>> Yes you can do that... However, there are two ways forward:
>>>
>>> 1) Drop and test with this patchset long enough to verify it's gone...
>>> 2) No dropping and get the more exact trace by reproducing, which can
>>> point out to tcp_retrans_try_collapse confirming the source of the
>>> bug or revealing yet another bug...
>>>
>>> The first one has one drawback, it cannot prove the fix very well since
>>> the bug could just not occur by chance... Path 2 would clearly show the
>>> place from where the problem originates because we will know that it got
>>> triggered! I personally would prefer path 2 but whether you want to go for
>>> that depends on the time you want to invest in it...
>>>
>>> ...I rediffed the tcp_verify_fackets patch too (below) just in case it
>>> would be something else in you case and you choose path 1 (put it on top
>>> of this patchset, applies with some offsets). In case the problem is gone,
>>> it shouldn't trigger and if it does, we'll have another bug caught.
>> I have a spare node so I'm starting 2) with the 3 patches you sent and that
>> last one which applied fine.
>
> Ah, that's path 1) then... Since you seem to have enough time, I would say
> that the path 1 is good as well and bugs unrelated to the fix will show up
> there too...
arg. yes. sorry for the confusion.
> I should have stated it explicitly that with path 2 those 3 patches should
> not be applied because the aim is not a fix but reproducal. Path 2 was
> intentionally left without the potentional fix as then nice backtrace
> informs when we can stop trying (which would hopefully occurred
> pretty soon) :-). But lets discard that path 2...
I have 2 spare nodes so i'll run both. 1) is on already without any issues
i'm just compiling 2)
I usually work on -mm, so what would be interesting for me is to have what you
need in net-2.6.24 which is getting pulled in -mm by andrew. then, if you need
an extra patch for verbosity, that's fine, i'll include it in my usual patchset.
Cheers,
C.
>> all of them on a fresh git pull of net-2.6.24
>
> That's fine, they're pretty well in sync (mm and net-2.6.24, and
> soon 2.6.24-rcs too).
>
^ permalink raw reply
* Re: [patch 3/3] git-net: sctp build fix (not for applying)
From: Vlad Yasevich @ 2007-10-03 13:50 UTC (permalink / raw)
To: akpm; +Cc: davem, netdev
In-Reply-To: <200710022104.l92L4dvJ022234@imap1.linux-foundation.org>
akpm@linux-foundation.org wrote:
> From: Andrew Morton <akpm@linux-foundation.org>
>
> net/sctp/sm_statetable.c:551: error: 'sctp_sf_tabort_8_4_8' undeclared here (not in a function)
>
Andrew, is the a result of the merge of net-2.6.24 with net-2.6?
That's the only way I see this happening.
>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> net/sctp/sm_statetable.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff -puN net/sctp/sm_statetable.c~git-net-sctp-hack net/sctp/sm_statetable.c
> --- a/net/sctp/sm_statetable.c~git-net-sctp-hack
> +++ a/net/sctp/sm_statetable.c
> @@ -527,8 +527,6 @@ static const sctp_sm_table_entry_t prsct
> /* SCTP_STATE_EMPTY */ \
> TYPE_SCTP_FUNC(sctp_sf_ootb), \
> /* SCTP_STATE_CLOSED */ \
> - TYPE_SCTP_FUNC(sctp_sf_tabort_8_4_8), \
^^^^^^^^^^^^^^^^^^^^
That should be changed to sctp_sf_ootb and then it'll compile. As is, the patch
is wrong.
Thanks
-vlad
> - /* SCTP_STATE_COOKIE_WAIT */ \
> TYPE_SCTP_FUNC(sctp_sf_discard_chunk), \
> /* SCTP_STATE_COOKIE_ECHOED */ \
> TYPE_SCTP_FUNC(sctp_sf_eat_auth), \
> _
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] Fallback to ipv4 if we try to add join IPv4 multicast group via ipv4-mapped address.
From: Dmitry Baryshkov @ 2007-10-03 13:26 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
In-Reply-To: <OFDC1D2E6E.2DCF75EA-ON88257368.0053513A-88257368.005586DC@us.ibm.com>
Hello,
David Stevens wrote:
> Dmitry,
> Good catch; a couple comments:
Thank you for the response.
>
>> struct ipv6_pinfo *np = inet6_sk(sk);
>> int err;
>> + int addr_type = ipv6_addr_type(addr);
>> +
>> + if (addr_type == IPV6_ADDR_MAPPED) {
>> + __be32 v4addr = addr->s6_addr32[3];
>> + struct ip_mreqn mreq;
>> + mreq.imr_multiaddr.s_addr = v4addr;
>> + mreq.imr_address.s_addr = INADDR_ANY;
>> + mreq.imr_ifindex = ifindex;
>> +
>> + return ip_mc_join_group(sk, &mreq);
>> + }
>
> ipv6_addr_type() returns a bitmask, so you should use:
>
> if (addr_type & IPV6_ADDR_MAPPED) {
I just c'n'pasted the code that checks for mapped addresses. In most
cases it's just ==, not bitmask operation.
>
> Also, you should have a blank line after the "mreq" declaration.
ok.
>
> Ditto for both in ipv6_mc_sock_drop().
> I don't expect the multicast source filtering interface will
> behave well for mapped addresses, either. The mapped multicast
> address won't appear to be a multicast address (and return
> error there), and all the source filters would have to be
> v4mapped addresses and modify the v4 source filters for this
> to do as you expect. So, there's more to it (and it may be a
> bit messy) to support mapped multicast addresses fully. I'll
> think about that part some more.
Didn't have time to test it throughly. I've only checked that call
succeeds and that all necessary igmp are sent. I hope, this weekend I'll
have more time to check.
--
With best wishes
Dmitry Baryshkov
^ permalink raw reply
* Re: [PATCH 2/3][NET_BATCH] net core use batching
From: jamal @ 2007-10-03 13:42 UTC (permalink / raw)
To: Bill Fink
Cc: David Miller, krkumar2, johnpol, herbert, kaber, shemminger,
jagana, Robert.Olsson, rick.jones2, xma, gaagaan, netdev, rdreier,
peter.p.waskiewicz.jr, mcarlson, jeff, mchan, general, kumarkr,
tgraf, randy.dunlap, sri
In-Reply-To: <20071003012929.d28f7cd8.billfink@mindspring.com>
On Wed, 2007-03-10 at 01:29 -0400, Bill Fink wrote:
> It does sound sensible. My own decidedly non-expert speculation
> was that the big 30 % performance hit right at 4 KB may be related
> to memory allocation issues or having to split the skb across
> multiple 4 KB pages.
plausible. But i also worry it could be 10 other things; example, could
it be the driver used? I noted in my udp test the oddity that turned out
to be tx coal parameter related.
In any case, I will attempt to run those tests later.
> And perhaps it only affected the single
> process case because with multiple processes lock contention may
> be a bigger issue and the xmit batching changes would presumably
> help with that. I am admittedly a novice when it comes to the
> detailed internals of TCP/skb processing, although I have been
> slowly slogging my way through parts of the TCP kernel code to
> try and get a better understanding, so I don't know if these
> thoughts have any merit.
You do bring up issues that need to be looked into and i will run those
tests.
Note, the effectiveness of batching becomes evident as the number of
flows grows. Actually, scratch that: It becomes evident if you can keep
the tx path busyed out to which multiple users running contribute. If i
can have a user per CPU with lots of traffic to send, i can create that
condition. It's a little boring in the scenario where the bottleneck is
the wire but it needs to be checked.
> BTW does anyone know of a good book they would recommend that has
> substantial coverage of the Linux kernel TCP code, that's fairly
> up-to-date and gives both an overall view of the code and packet
> flow as well as details on individual functions and algorithms,
> and hopefully covers basic issues like locking and synchronization,
> concurrency of different parts of the stack, and memory allocation.
> I have several books already on Linux kernel and networking internals,
> but they seem to only cover the IP (and perhaps UDP) portions of the
> network stack, and none have more than a cursory reference to TCP.
> The most useful documentation on the Linux TCP stack that I have
> found thus far is some of Dave Miller's excellent web pages and
> a few other web references, but overall it seems fairly skimpy
> for such an important part of the Linux network code.
Reading books or magazines may end up busying you out with some small
gains of knowledge at the end. They tend to be outdated fast. My advice
is if you start with a focus on one thing, watch the patches that fly
around on that area and learn that way. Read the code to further
understand things then ask questions when its not clear. Other folks may
have different views. The other way to do it is pick yourself some task
to either add or improve something and get your hands dirty that way.
> It would be good to see some empirical evidence that there aren't
> any unforeseen gotchas for larger packet sizes, that at least the
> same level of performance can be obtained with no greater CPU
> utilization.
Reasonable - I will try with 9K after i move over to the new tree from
Dave and make sure nothing else broke in the previous tests.
And when all looks good, i will move to TCP.
> > [1] On average i spend 10x more time performance testing and analysing
> > results than writting code.
>
> As you have written previously, and I heartily agree with, this is a
> very good practice for developing performance enhancement patches.
To give you a perspective, the results i posted were each run 10
iterations per packet size per kernel. Each run is 60 seconds long. I
think i am past that stage for resolving or fixing anything for UDP or
pktgen, but i need to keep checking for any new regressions when Dave
updates his tree. Now multiply that by 5 packet sizes (I am going to add
2 more) and multiply that by 3-4 kernels. Then add the time it takes to
sift through the data and collect it then analyze it and go back to the
drawing table when something doesnt look right. Essentially, it needs a
weekend ;->
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-2.6.24 0/3]: More TCP fixes
From: Ilpo Järvinen @ 2007-10-03 13:19 UTC (permalink / raw)
To: Cedric Le Goater; +Cc: David Miller, Netdev
In-Reply-To: <47038FAB.9020106@free.fr>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2003 bytes --]
On Wed, 3 Oct 2007, Cedric Le Goater wrote:
> Ilpo Järvinen wrote:
> > On Wed, 3 Oct 2007, Cedric Le Goater wrote:
> >
> >> I'm dropping the previous patches you sent me and switching to this patchset.
> >> right ?
> >
> > Yes you can do that... However, there are two ways forward:
> >
> > 1) Drop and test with this patchset long enough to verify it's gone...
> > 2) No dropping and get the more exact trace by reproducing, which can
> > point out to tcp_retrans_try_collapse confirming the source of the
> > bug or revealing yet another bug...
> >
> > The first one has one drawback, it cannot prove the fix very well since
> > the bug could just not occur by chance... Path 2 would clearly show the
> > place from where the problem originates because we will know that it got
> > triggered! I personally would prefer path 2 but whether you want to go for
> > that depends on the time you want to invest in it...
> >
> > ...I rediffed the tcp_verify_fackets patch too (below) just in case it
> > would be something else in you case and you choose path 1 (put it on top
> > of this patchset, applies with some offsets). In case the problem is gone,
> > it shouldn't trigger and if it does, we'll have another bug caught.
>
> I have a spare node so I'm starting 2) with the 3 patches you sent and that
> last one which applied fine.
Ah, that's path 1) then... Since you seem to have enough time, I would say
that the path 1 is good as well and bugs unrelated to the fix will show up
there too...
I should have stated it explicitly that with path 2 those 3 patches should
not be applied because the aim is not a fix but reproducal. Path 2 was
intentionally left without the potentional fix as then nice backtrace
informs when we can stop trying (which would hopefully occurred
pretty soon) :-). But lets discard that path 2...
> all of them on a fresh git pull of net-2.6.24
That's fine, they're pretty well in sync (mm and net-2.6.24, and
soon 2.6.24-rcs too).
--
i.
^ permalink raw reply
* Re: [PATCH][TG3]Some cleanups
From: jamal @ 2007-10-03 13:18 UTC (permalink / raw)
To: Michael Chan; +Cc: Matt Carlson, netdev
In-Reply-To: <1191368033.5961.22.camel@dell>
On Tue, 2007-02-10 at 16:33 -0700, Michael Chan wrote:
> Seems ok to me. I think we should make it more clear that we're
> skipping over the VLAN tag:
>
> (struct tg3_tx_cbdata *)&((__skb)->cb[sizeof(struct vlan_skb_tx_cookie)])
>
Will do - thanks Michael.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH][E1000E] some cleanups
From: jamal @ 2007-10-03 13:18 UTC (permalink / raw)
To: Kok, Auke; +Cc: netdev
In-Reply-To: <47028351.7090509@intel.com>
On Tue, 2007-02-10 at 10:43 -0700, Kok, Auke wrote:
> the description of this patch is rather misleading, and the title certainly too.
That was fast - you said weeks, not days;->
> Can you resend this with a bit more elaborate explanation as to why the cb code is
> relevant to use here? Not only do I need to understand this, but others might want
> to as well later on ;)
I am probably repeating something youve seen/know already.
The cleanup is to break up the code so it is functionally more readable
from a perspective of the 4 distinct parts in ->hard_start_xmit():
a) packet formatting (example: vlan, mss, descriptor counting, etc.)
b) chip-specific formatting
c) enqueueing the packet on a DMA ring
d) IO operations to complete packet transmit, tell DMA engine to chew
on, tx completion interrupts, set last tx time, etc.
Each of those steps sitting in different functions accumulates state
that is used in the next steps. cb stores this state because it a
scratchpad the driver owns. You could create some other structure and
pass it around the iteration, but why waste more bytes.
I could stop there with the explanation, but let me go on .. ;->
>From a secondary angle, remember i am pulling these patches out of my
batching work. Thats how we started this discussion ;-> I would like,
once converted the driver to remove LLTX, to do #a without holding the
tx lock. This stands on its own even without batching. Then of course,
once all this is in such good shape it makes it easier to add the
batching code because i could reuse the now functionalized steps.
I hope that provides reasonable and good explanation ;->
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-2.6.24 0/3]: More TCP fixes
From: Cedric Le Goater @ 2007-10-03 12:48 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: David Miller, Netdev
In-Reply-To: <Pine.LNX.4.64.0710031503220.27745@kivilampi-30.cs.helsinki.fi>
Ilpo Järvinen wrote:
> On Wed, 3 Oct 2007, Cedric Le Goater wrote:
>
>> Ilpo Järvinen wrote:
>>> Sacktag fastpath_cnt_hint seems to be very tricky to get right...
>>> I suppose this one fixes Cedric's case. I cannot say for sure
>>> until there is something more definite indication of
>>> tcp_retrans_try_collapse origin than what the simple late WARN_ON
>>> gave for us. ...Especially since it's non-trivial to have skb
>>> hint "correctly" positioned in the write_queue while still ending
>>> up calling that function. However, considering how difficult it
>>> seems to be for Cedric to reproduce, it might well be this one.
>>>
>>> In addition, I noticed another reset which wasn't previously
>>> converted to WARN_ON, so doing that now. Boot + simple xfer
>>> tested. Please apply to net-2.6.24.
>> I'm dropping the previous patches you sent me and switching to this patchset.
>> right ?
>
> Yes you can do that... However, there are two ways forward:
>
> 1) Drop and test with this patchset long enough to verify it's gone...
> 2) No dropping and get the more exact trace by reproducing, which can
> point out to tcp_retrans_try_collapse confirming the source of the
> bug or revealing yet another bug...
>
> The first one has one drawback, it cannot prove the fix very well since
> the bug could just not occur by chance... Path 2 would clearly show the
> place from where the problem originates because we will know that it got
> triggered! I personally would prefer path 2 but whether you want to go for
> that depends on the time you want to invest in it...
>
> ...I rediffed the tcp_verify_fackets patch too (below) just in case it
> would be something else in you case and you choose path 1 (put it on top
> of this patchset, applies with some offsets). In case the problem is gone,
> it shouldn't trigger and if it does, we'll have another bug caught.
I have a spare node so I'm starting 2) with the 3 patches you sent and that
last one which applied fine. all of them on a fresh git pull of net-2.6.24
> Anyway, thanks for ccing right persons and netdev right from the
> beginning.
thanks to git ! :)
C.
^ permalink raw reply
* Re: [PATCH net-2.6.24 0/3]: More TCP fixes
From: Ilpo Järvinen @ 2007-10-03 12:34 UTC (permalink / raw)
To: Cedric Le Goater; +Cc: David Miller, Netdev
In-Reply-To: <470383D4.9060307@free.fr>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 10192 bytes --]
On Wed, 3 Oct 2007, Cedric Le Goater wrote:
> Ilpo Järvinen wrote:
> > Sacktag fastpath_cnt_hint seems to be very tricky to get right...
> > I suppose this one fixes Cedric's case. I cannot say for sure
> > until there is something more definite indication of
> > tcp_retrans_try_collapse origin than what the simple late WARN_ON
> > gave for us. ...Especially since it's non-trivial to have skb
> > hint "correctly" positioned in the write_queue while still ending
> > up calling that function. However, considering how difficult it
> > seems to be for Cedric to reproduce, it might well be this one.
> >
> > In addition, I noticed another reset which wasn't previously
> > converted to WARN_ON, so doing that now. Boot + simple xfer
> > tested. Please apply to net-2.6.24.
>
> I'm dropping the previous patches you sent me and switching to this patchset.
> right ?
Yes you can do that... However, there are two ways forward:
1) Drop and test with this patchset long enough to verify it's gone...
2) No dropping and get the more exact trace by reproducing, which can
point out to tcp_retrans_try_collapse confirming the source of the
bug or revealing yet another bug...
The first one has one drawback, it cannot prove the fix very well since
the bug could just not occur by chance... Path 2 would clearly show the
place from where the problem originates because we will know that it got
triggered! I personally would prefer path 2 but whether you want to go for
that depends on the time you want to invest in it...
...I rediffed the tcp_verify_fackets patch too (below) just in case it
would be something else in you case and you choose path 1 (put it on top
of this patchset, applies with some offsets). In case the problem is gone,
it shouldn't trigger and if it does, we'll have another bug caught.
Anyway, thanks for ccing right persons and netdev right from the
beginning.
--
i.
include/net/tcp.h | 3 +
net/ipv4/tcp_input.c | 25 +++++++++---
net/ipv4/tcp_ipv4.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_output.c | 6 ++-
4 files changed, 130 insertions(+), 7 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 991ccdc..54a0d91 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -43,6 +43,9 @@
#include <linux/seq_file.h>
+extern void tcp_verify_fackets(struct sock *sk);
+extern void tcp_print_queue(struct sock *sk);
+
extern struct inet_hashinfo tcp_hashinfo;
extern atomic_t tcp_orphan_count;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 87c9ef5..93bdc20 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1140,7 +1140,7 @@ static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
return dup_sack;
}
-static int
+int
tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_una)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -1160,8 +1160,10 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
int first_sack_index;
if (!tp->sacked_out) {
- if (WARN_ON(tp->fackets_out))
+ if (WARN_ON(tp->fackets_out)) {
tp->fackets_out = 0;
+ tcp_print_queue(sk);
+ }
tp->highest_sack = tp->snd_una;
}
prior_fackets = tp->fackets_out;
@@ -1421,6 +1423,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
}
}
+ tcp_verify_fackets(sk);
/* Check for lost retransmit. This superb idea is
* borrowed from "ratehalving". Event "C".
@@ -1633,13 +1636,14 @@ void tcp_enter_frto(struct sock *sk)
tcp_set_ca_state(sk, TCP_CA_Disorder);
tp->high_seq = tp->snd_nxt;
tp->frto_counter = 1;
+ tcp_verify_fackets(sk);
}
/* Enter Loss state after F-RTO was applied. Dupack arrived after RTO,
* which indicates that we should follow the traditional RTO recovery,
* i.e. mark everything lost and do go-back-N retransmission.
*/
-static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
+void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
{
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *skb;
@@ -1676,6 +1680,7 @@ static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
}
}
tcp_verify_left_out(tp);
+ tcp_verify_fackets(sk);
tp->snd_cwnd = tcp_packets_in_flight(tp) + allowed_segments;
tp->snd_cwnd_cnt = 0;
@@ -1754,6 +1759,7 @@ void tcp_enter_loss(struct sock *sk, int how)
}
}
tcp_verify_left_out(tp);
+ tcp_verify_fackets(sk);
tp->reordering = min_t(unsigned int, tp->reordering,
sysctl_tcp_reordering);
@@ -2309,7 +2315,7 @@ static void tcp_mtup_probe_success(struct sock *sk, struct sk_buff *skb)
* It does _not_ decide what to send, it is made in function
* tcp_xmit_retransmit_queue().
*/
-static void
+void
tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
{
struct inet_connection_sock *icsk = inet_csk(sk);
@@ -2323,8 +2329,11 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
if (!tp->packets_out)
tp->sacked_out = 0;
- if (WARN_ON(!tp->sacked_out && tp->fackets_out))
+ if (WARN_ON(!tp->sacked_out && tp->fackets_out)) {
+ printk(KERN_ERR "TCP %d\n", tcp_is_reno(tp));
+ tcp_print_queue(sk);
tp->fackets_out = 0;
+ }
/* Now state machine starts.
* A. ECE, hence prohibit cwnd undoing, the reduction is required. */
@@ -2334,6 +2343,8 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
/* B. In all the states check for reneging SACKs. */
if (tp->sacked_out && tcp_check_sack_reneging(sk))
return;
+
+ tcp_verify_fackets(sk);
/* C. Process data loss notification, provided it is valid. */
if ((flag&FLAG_DATA_LOST) &&
@@ -2573,7 +2584,7 @@ static u32 tcp_tso_acked(struct sock *sk, struct sk_buff *skb)
* is before the ack sequence we can discard it as it's confirmed to have
* arrived at the other end.
*/
-static int tcp_clean_rtx_queue(struct sock *sk, s32 *seq_rtt_p)
+int tcp_clean_rtx_queue(struct sock *sk, s32 *seq_rtt_p)
{
struct tcp_sock *tp = tcp_sk(sk);
const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -2695,6 +2706,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, s32 *seq_rtt_p)
ca_ops->pkts_acked(sk, pkts_acked, rtt_us);
}
}
+ tcp_verify_fackets(sk);
+
#if FASTRETRANS_DEBUG > 0
BUG_TRAP((int)tp->sacked_out >= 0);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 7fed0a6..b5877d0 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -108,6 +108,109 @@ struct inet_hashinfo __cacheline_aligned tcp_hashinfo = {
.lhash_wait = __WAIT_QUEUE_HEAD_INITIALIZER(tcp_hashinfo.lhash_wait),
};
+void tcp_print_queue(struct sock *sk)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct sk_buff *skb;
+ char s[50+1];
+ char i[50+1];
+ int idx = 0;
+ u32 hs = tp->highest_sack;
+
+ if (!tp->sacked_out)
+ hs = tp->snd_una;
+
+ tcp_for_write_queue(skb, sk) {
+ if (skb == tcp_send_head(sk))
+ break;
+
+ if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED) {
+ if (skb->len < tp->mss_cache)
+ s[idx] = 's';
+ else
+ s[idx] = 'S';
+ } else {
+ s[idx] = '-';
+ }
+ if ((TCP_SKB_CB(skb)->seq == hs) && (tp->fastpath_skb_hint == skb))
+ i[idx] = 'x';
+ else if (tp->fastpath_skb_hint == skb)
+ i[idx] = 'f';
+ else if (TCP_SKB_CB(skb)->seq == hs)
+ i[idx] = 'h';
+ else
+ i[idx] = ' ';
+
+ if (++idx >= 50) {
+ s[idx] = 0;
+ i[idx] = 0;
+ printk(KERN_ERR "TCP wq(s) %s\n", s);
+ printk(KERN_ERR "TCP wq(i) %s\n", i);
+ idx = 0;
+ }
+ }
+ if (idx) {
+ s[idx] = '<';
+ s[idx+1] = 0;
+ i[idx] = '<';
+ i[idx+1] = 0;
+ printk(KERN_ERR "TCP wq(s) %s\n", s);
+ printk(KERN_ERR "TCP wq(i) %s\n", i);
+ }
+ printk(KERN_ERR "s%u f%u (%u) p%u seq: su%u hs%u sn%u (%u)\n",
+ tp->sacked_out, tp->fackets_out, tp->fastpath_cnt_hint,
+ tp->packets_out,
+ tp->snd_una, tp->highest_sack, tp->snd_nxt,
+ ((tp->fastpath_skb_hint == NULL) ? 0 :
+ TCP_SKB_CB(tp->fastpath_skb_hint)->seq));
+}
+
+void tcp_verify_fackets(struct sock *sk)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct sk_buff *skb;
+ u32 fackets = 0;
+ int hisack_valid = 0;
+ int err = 0;
+
+ if (tcp_is_reno(tp))
+ return;
+
+ if (!tp->sacked_out) {
+ if (WARN_ON(tp->fackets_out))
+ err = 1;
+ else if (tp->fastpath_skb_hint == NULL)
+ return;
+ }
+
+ /* ...expensive processing here... */
+ tcp_for_write_queue(skb, sk) {
+ if (skb == tcp_send_head(sk))
+ break;
+
+ if (tp->sacked_out && (TCP_SKB_CB(skb)->seq == tp->highest_sack)) {
+ hisack_valid = 1;
+ if (WARN_ON(tp->fackets_out != fackets + tcp_skb_pcount(skb)))
+ err = 1;
+ }
+
+ if (skb == tp->fastpath_skb_hint)
+ if (WARN_ON(fackets != tp->fastpath_cnt_hint))
+ err = 1;
+
+ if (WARN_ON((fackets > tp->fackets_out) && (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED)))
+ err = 1;
+
+ fackets += tcp_skb_pcount(skb);
+ }
+
+ if (WARN_ON(tp->sacked_out && !hisack_valid))
+ err = 1;
+
+ if (err)
+ tcp_print_queue(sk);
+}
+
static int tcp_v4_get_port(struct sock *sk, unsigned short snum)
{
return inet_csk_get_port(&tcp_hashinfo, sk, snum,
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 5329675..3aba96a 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -773,6 +773,8 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len, unsigned int mss
tcp_verify_left_out(tp);
}
tcp_adjust_fackets_out(tp, skb, diff);
+
+ tcp_verify_fackets(sk);
}
/* Link BUFF into the send queue. */
@@ -1688,7 +1690,7 @@ u32 __tcp_select_window(struct sock *sk)
}
/* Attempt to collapse two adjacent SKB's during retransmission. */
-static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int mss_now)
+void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int mss_now)
{
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *next_skb = tcp_write_queue_next(sk, skb);
@@ -1766,6 +1768,8 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
tp->fastpath_cnt_hint -= tcp_skb_pcount(skb);
}
+ tcp_verify_fackets(sk);
+
sk_stream_free_skb(sk, next_skb);
}
}
--
1.5.0.6
^ permalink raw reply related
* Re: [PATCH net-2.6.24 0/3]: More TCP fixes
From: Cedric Le Goater @ 2007-10-03 11:58 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: David Miller, netdev
In-Reply-To: <1191409218982-git-send-email-ilpo.jarvinen@helsinki.fi>
Hello Ilpo !
Ilpo Järvinen wrote:
> Hi Dave,
>
> Sacktag fastpath_cnt_hint seems to be very tricky to get right...
> I suppose this one fixes Cedric's case. I cannot say for sure
> until there is something more definite indication of
> tcp_retrans_try_collapse origin than what the simple late WARN_ON
> gave for us. ...Especially since it's non-trivial to have skb
> hint "correctly" positioned in the write_queue while still ending
> up calling that function. However, considering how difficult it
> seems to be for Cedric to reproduce, it might well be this one.
>
> In addition, I noticed another reset which wasn't previously
> converted to WARN_ON, so doing that now. Boot + simple xfer
> tested. Please apply to net-2.6.24.
I'm dropping the previous patches you sent me and switching to this patchset.
right ?
Thanks,
C.
^ permalink raw reply
* [PATCH 3/3] [TCP]: "Annotate" another fackets_out state reset
From: Ilpo Järvinen @ 2007-10-03 11:00 UTC (permalink / raw)
To: David Miller, Cedric Le Goater; +Cc: netdev
In-Reply-To: <11914092181600-git-send-email-ilpo.jarvinen@helsinki.fi>
This should no longer be necessary because fackets_out is
accurate. It indicates bugs elsewhere, thus report it.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e22ffe7..87c9ef5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1160,7 +1160,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
int first_sack_index;
if (!tp->sacked_out) {
- tp->fackets_out = 0;
+ if (WARN_ON(tp->fackets_out))
+ tp->fackets_out = 0;
tp->highest_sack = tp->snd_una;
}
prior_fackets = tp->fackets_out;
--
1.5.0.6
^ permalink raw reply related
* [PATCH net-2.6.24 0/3]: More TCP fixes
From: Ilpo Järvinen @ 2007-10-03 11:00 UTC (permalink / raw)
To: David Miller, Cedric Le Goater; +Cc: netdev
Hi Dave,
Sacktag fastpath_cnt_hint seems to be very tricky to get right...
I suppose this one fixes Cedric's case. I cannot say for sure
until there is something more definite indication of
tcp_retrans_try_collapse origin than what the simple late WARN_ON
gave for us. ...Especially since it's non-trivial to have skb
hint "correctly" positioned in the write_queue while still ending
up calling that function. However, considering how difficult it
seems to be for Cedric to reproduce, it might well be this one.
In addition, I noticed another reset which wasn't previously
converted to WARN_ON, so doing that now. Boot + simple xfer
tested. Please apply to net-2.6.24.
--
i.
^ permalink raw reply
* [PATCH 2/3] [TCP]: Comment fastpath_cnt_hint off-by-one trap
From: Ilpo Järvinen @ 2007-10-03 11:00 UTC (permalink / raw)
To: David Miller, Cedric Le Goater; +Cc: netdev
In-Reply-To: <11914092183725-git-send-email-ilpo.jarvinen@helsinki.fi>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/linux/tcp.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index f8cf090..9ff456e 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -343,7 +343,8 @@ struct tcp_sock {
struct sk_buff *forward_skb_hint;
struct sk_buff *fastpath_skb_hint;
- int fastpath_cnt_hint;
+ int fastpath_cnt_hint; /* Lags behind by current skb's pcount
+ * compared to respective fackets_out */
int lost_cnt_hint;
int retransmit_cnt_hint;
--
1.5.0.6
^ permalink raw reply related
* [PATCH 1/3] [TCP]: Fix two off-by-one errors in fackets_out adjusting logic
From: Ilpo Järvinen @ 2007-10-03 11:00 UTC (permalink / raw)
To: David Miller, Cedric Le Goater; +Cc: netdev
In-Reply-To: <1191409218982-git-send-email-ilpo.jarvinen@helsinki.fi>
1) Passing wrong skb to tcp_adjust_fackets_out could corrupt
fastpath_cnt_hint as tcp_skb_pcount(next_skb) is not included
to it if hint points exactly to the next_skb (it's lagging
behind, see sacktag).
2) When fastpath_skb_hint is put backwards to avoid dangling
skb reference, the skb's pcount must also be removed from count
(not included like above).
Reported by Cedric Le Goater <legoater@free.fr>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_output.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 6199abe..5329675 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1755,14 +1755,16 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
if (tcp_is_reno(tp) && tp->sacked_out)
tcp_dec_pcount_approx(&tp->sacked_out, next_skb);
- tcp_adjust_fackets_out(tp, skb, tcp_skb_pcount(next_skb));
+ tcp_adjust_fackets_out(tp, next_skb, tcp_skb_pcount(next_skb));
tp->packets_out -= tcp_skb_pcount(next_skb);
/* changed transmit queue under us so clear hints */
tcp_clear_retrans_hints_partial(tp);
/* manually tune sacktag skb hint */
- if (tp->fastpath_skb_hint == next_skb)
+ if (tp->fastpath_skb_hint == next_skb) {
tp->fastpath_skb_hint = skb;
+ tp->fastpath_cnt_hint -= tcp_skb_pcount(skb);
+ }
sk_stream_free_skb(sk, next_skb);
}
--
1.5.0.6
^ permalink raw reply related
* net-2.6.24 plans
From: David Miller @ 2007-10-03 9:36 UTC (permalink / raw)
To: netdev
I'm a bit behind after investigating the TCP performance issues that
turned out to be HW specific problems. It's a bit of a
dissapointment, I thought maybe there was a cool bug to fix in TCP :-)
Anyways, that means there are patches backlogged in my inbox and it is
also about time to do the hopefully last rebase of the net-2.6.24
tree.
I merged in Jeff Garzik's and John Linville's latest and I'm running
the current tree on my workstation most of today with good results so
far.
Linus should release the final 2.6.23 very soon, let's kind of assume
it will happen over the next 3 or 4 days.
That means we need to bear down for the merge. I plan to commit my
Neptune driver in it's current state, and that's the last new feature
going in.
You can help make the merge go swimmingly by picking some nagging
issue you noticed and track it down. If you can figure out why
something happens but can't or don't have time to come up with
a fix, report what you've discovered.
If you can provide the fix too, all the better.
That's how I get backlogged, I'm working on A and notice some problem
with B, then I refuse to go back to A until I bring closure to B. :)
^ permalink raw reply
* Re: kernel 2.4 vs 2.6 Traffic Controller performance
From: Sonny @ 2007-10-03 8:31 UTC (permalink / raw)
To: netdev
In-Reply-To: <47033E10.6090505@cosmosbay.com>
On 10/3/07, Eric Dumazet <dada1@cosmosbay.com> wrote:
> Sonny a écrit :
> > Hello
> > This is a repost, there seems to have a misunderstanding before.
> >
> > I hope this is the right place to ask this. Does any know if there is a
> > substantial difference in the performance of the traffic controller
> > between kernel 2.4 and 2.6. We tested it using 1 iperf server and use
> > 250 and 500 clients, altering the burst.
> >
> > This is the set-up:
> > iperf client - router (w/ traffic controller) - iperf server
> >
> > We use the top command inside the router to check the idle time of our
> > router to see this. The results we got from the 2.4 kernel shows
> > around 65-70% idle time while the 2.6 shows
> > 60-65% idle time. We tried to use MRTG and we're not getting any
> > results either. We want to know if we could improve the bandwidth by
> > upgrading the kernel, else we would have to get a new bandwidth
> > manager. Have anyone performed a similar test or can suggest a better
> > way to do this. Thanks in advance.
> > -
> Hi Sonny
>
> I am not sure what you are asking here. 65-70% idle time (or 60-65%) is fine.
>
> 2.6 is also not very meaningfull, there are a lot of changes between 2.6.0 and
> 2.6.23 :)
>
we're using 2.6.22
> Why should you upgrade kernel ?
we would like to test the difference bet 2 kernels performance
> What bandwidth do you handle ?
10 mbps
> What kind of platform is it ? (a new kernel wont help much if its a real old
> machine, or old NICs)
it's a P IV 2.8 GHz HT with 512 MB
>
> You seem to have some bandwidth problem but focus on cpu affairs...
Bandwidth is not a problem, we can get 10mbps without a hitch. But we
would like to know the scalability on the CPU vs the number of
clients. So far, for both kernels, we're getting 50% CPU utilization
using 500 clients and 384 burst kbps each.
^ permalink raw reply
* Re: [PATCH] sky2: jumbo frame regression fix
From: iank @ 2007-10-03 8:29 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: pomac, Jeff Garzik, Linux-kernel, netdev
In-Reply-To: <20071002215914.13d1e8cf@freepuppy.rosehill>
On Tue, Oct 02, 2007 at 09:59:14PM -0700, Stephen Hemminger wrote:
> On Wed, 03 Oct 2007 03:34:34 +0200
> Ian Kumlien <pomac@vapor.com> wrote:
>
> > On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
> > > Remove unneeded check that caused problems with jumbo frame sizes.
> > > The check was recently added and is wrong.
> > > When using jumbo frames the sky2 driver does fragmentation, so
> > > rx_data_size is less than mtu.
> >
> > Confirmed working.
> >
> > Now running with 9k mtu with no errors, =)
> >
> > It also seems that the FIFO bug was the one that affected me before,
> > damn odd race that one.
>
> Does the workaround (forced reset work). Ian, you are the first person to
> report triggering it. I haven't found a way to make it happen.
> What combination of flow control and speeds are you using?
I forgot to add, last time was -rc8-git2 or 3 and using Westwood flow
control.
> --
> Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: tcp bw in 2.6
From: David Miller @ 2007-10-03 8:02 UTC (permalink / raw)
To: lm; +Cc: rick.jones2, torvalds, herbert, wscott, netdev
In-Reply-To: <20071002223644.GA14331@bitmover.com>
From: lm@bitmover.com (Larry McVoy)
Date: Tue, 2 Oct 2007 15:36:44 -0700
> On Tue, Oct 02, 2007 at 03:32:16PM -0700, David Miller wrote:
> > I'm starting to have a theory about what the bad case might
> > be.
> >
> > A strong sender going to an even stronger receiver which can
> > pull out packets into the process as fast as they arrive.
> > This might be part of what keeps the receive window from
> > growing.
>
> I can back you up on that. When I straced the receiving side that goes
> slowly, all the reads were short, like 1-2K. The way that works the
> reads were a lot larger as I recall.
My issue turns out to be hardware specific too.
The two Broadcom 5714 onboard NICs on my Niagara t1000 give bad packet
receive performance for some reason, the other two which are Broadcom
5704's are perfectly fine. I'll figure out what the problem is,
probably some misprogramed register in either the chip or the bridge
it's behind.
The UDP stream test of netperf is great for isolating TCP/TSO vs.
hardware issues. If you can't saturate the pipe or the cpu with
the UDP stream test, it's likely a hardware issue.
The cpu utilization and service demand numbers provided, on both
send and receive, are really useful for diagnosing problems like
this.
Rick deserves several beers for his work on this cool toy. :)
^ permalink raw reply
* Re: [PATCH] sky2: jumbo frame regression fix
From: Ian Kumlien @ 2007-10-03 7:37 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jeff Garzik, Linux-kernel, netdev
In-Reply-To: <20071002215914.13d1e8cf@freepuppy.rosehill>
[-- Attachment #1: Type: text/plain, Size: 1701 bytes --]
On tis, 2007-10-02 at 21:59 -0700, Stephen Hemminger wrote:
> On Wed, 03 Oct 2007 03:34:34 +0200
> Ian Kumlien <pomac@vapor.com> wrote:
>
> > On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
> > > Remove unneeded check that caused problems with jumbo frame sizes.
> > > The check was recently added and is wrong.
> > > When using jumbo frames the sky2 driver does fragmentation, so
> > > rx_data_size is less than mtu.
> >
> > Confirmed working.
> >
> > Now running with 9k mtu with no errors, =)
> >
> > It also seems that the FIFO bug was the one that affected me before,
> > damn odd race that one.
>
> Does the workaround (forced reset work). Ian, you are the first person to
> report triggering it. I haven't found a way to make it happen.
> What combination of flow control and speeds are you using?
Yes it works, it's the problem i had all along =)
As to how to make it happen thats a bit harder...
To me it seems like it's a combination of several connections and
somewhat high bandwidth but you have to send data for it to happen...
To me it usually happens when seeding files via Bittorrent, but it seems
like it has to be somewhat special circumstances to actually trigger it.
I use jumbo frames, my lan is gigabit, to my firewall. From the firewall
it's common 1500 mtu 100mbit and i doubt that this has anything to do
with it (if it's not a 'number of frames that can be stored' problem and
thus the mtu limits it to a really small value making it easier to
trigger)
Well, thats my thoughts atleast but then i just got up after having
slept 5 hours, so =)
--
Ian Kumlien <pomac () vapor ! com> -- http://pomac.netswarm.net
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: tcp bw in 2.6
From: Bill Fink @ 2007-10-03 7:19 UTC (permalink / raw)
To: Rick Jones; +Cc: Larry McVoy, Linus Torvalds, davem, wscott, netdev
In-Reply-To: <47027C63.803@hp.com>
Tangential aside:
On Tue, 02 Oct 2007, Rick Jones wrote:
> *) depending on the quantity of CPU around, and the type of test one is running,
> results can be better/worse depending on the CPU to which you bind the
> application. Latency tends to be best when running on the same core as takes
> interrupts from the NIC, bulk transfer can be better when running on a different
> core, although generally better when a different core on the same chip. These
> days the throughput stuff is more easily seen on 10G, but the netperf service
> demand changes are still visible on 1G.
Interesting. I was going to say that I've generally had the opposite
experience when it comes to bulk data transfers, which is what I would
expect due to CPU caching effects, but that perhaps it's motherboard/NIC/
driver dependent. But in testing I just did I discovered it's even
MTU dependent (most of my normal testing is always with 9000-byte
jumbo frames).
With Myricom 10-GigE NICs, NIC interrupts on CPU 0 and nuttcp app
running on CPU 1 (both transmit and receive sides), and using 9000-byte
jumbo frames:
[root@lang2 ~]# nuttcp -w10m 192.168.88.16
10078.5000 MB / 10.02 sec = 8437.5396 Mbps 100 %TX 99 %RX
With Myricom 10-GigE NICs, and both NIC interrupts and nuttcp app
on CPU 0 (both transmit and receive sides), again using 9000-byte
jumbo frames:
[root@lang2 ~]# nuttcp -w10m 192.168.88.16
11817.8750 MB / 10.00 sec = 9909.7537 Mbps 100 %TX 74 %RX
Same tests repeated with standard 1500-byte Ethernet MTU:
With Myricom 10-GigE NICs, NIC interrupts on CPU 0 and nuttcp app
running on CPU 1 (both transmit and receive sides), and using
standard 1500-byte Ethernet MTU:
[root@lang2 ~]# nuttcp -M1460 -w10m 192.168.88.16
5685.9375 MB / 10.00 sec = 4768.0951 Mbps 99 %TX 98 %RX
With Myricom 10-GigE NICs, and both NIC interrupts and nuttcp app
on CPU 0 (both transmit and receive sides), again using standard
1500-byte Ethernet MTU:
[root@lang2 ~]# nuttcp -M1460 -w10m 192.168.88.16
4974.0625 MB / 10.03 sec = 4161.6015 Mbps 100 %TX 100 %RX
Now back to your regularly scheduled programming. :-)
-Bill
^ permalink raw reply
* Re: [PATCH 5/7] CAN: Add virtual CAN netdevice driver
From: Oliver Hartkopp @ 2007-10-03 7:06 UTC (permalink / raw)
To: David Miller, acme
Cc: urs, netdev, kaber, tglx, yoshfuji, ebiederm, oliver.hartkopp,
urs.thuermann
In-Reply-To: <20071002.145034.15615464.davem@davemloft.net>
David Miller wrote:
> From: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Date: Tue, 2 Oct 2007 18:43:25 -0300
>
>
>> I think that helping ctags to find the definition for the debug variable
>> to see, for instance, if it is a bitmask or a boolean without having to
>> chose from tons of 'debug' variables is a good thing.
>>
>
> I completely agree.
>
OK. No problem if it's helpful. We'll change debug to vcan_debug.
Thanks.
Oliver
^ permalink raw reply
* Re: kernel 2.4 vs 2.6 Traffic Controller performance
From: Eric Dumazet @ 2007-10-03 7:00 UTC (permalink / raw)
To: Sonny; +Cc: netdev
In-Reply-To: <9dbec86d0710022314o6db2b0baua6db543d52f312b6@mail.gmail.com>
Sonny a écrit :
> Hello
> This is a repost, there seems to have a misunderstanding before.
>
> I hope this is the right place to ask this. Does any know if there is a
> substantial difference in the performance of the traffic controller
> between kernel 2.4 and 2.6. We tested it using 1 iperf server and use
> 250 and 500 clients, altering the burst.
>
> This is the set-up:
> iperf client - router (w/ traffic controller) - iperf server
>
> We use the top command inside the router to check the idle time of our
> router to see this. The results we got from the 2.4 kernel shows
> around 65-70% idle time while the 2.6 shows
> 60-65% idle time. We tried to use MRTG and we're not getting any
> results either. We want to know if we could improve the bandwidth by
> upgrading the kernel, else we would have to get a new bandwidth
> manager. Have anyone performed a similar test or can suggest a better
> way to do this. Thanks in advance.
> -
Hi Sonny
I am not sure what you are asking here. 65-70% idle time (or 60-65%) is fine.
2.6 is also not very meaningfull, there are a lot of changes between 2.6.0 and
2.6.23 :)
Why should you upgrade kernel ?
What bandwidth do you handle ?
What kind of platform is it ? (a new kernel wont help much if its a real old
machine, or old NICs)
You seem to have some bandwidth problem but focus on cpu affairs...
^ 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