* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Sridhar Samudrala @ 2009-08-26 22:11 UTC (permalink / raw)
To: Christoph Lameter
Cc: David Stevens, David S. Miller, Eric Dumazet, netdev, niv
In-Reply-To: <alpine.DEB.1.10.0908261506590.9933@gentwo.org>
On Wed, 2009-08-26 at 15:09 -0400, Christoph Lameter wrote:
> On Wed, 26 Aug 2009, Sridhar Samudrala wrote:
>
> > > They are reported for IP and UDP.
> > Not clear what you meant by this.
>
> The SNMP and UDP statistics show the loss. qdisc level does not show the
> loss.
> > > root@rd-strategy3-deb64:/home/clameter#tc -s qdisc show
> > > qdisc pfifo_fast 0: dev eth0 root bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1
> > > 1 1 1 1
> > > Sent 6208 bytes 64 pkt (dropped 0, overlimits 0 requeues 0)
> > > rate 0bit 0pps backlog 0b 0p requeues 0
> >
> > Even the Sent count seems to be too low. Are you looking at the right
> > device?
>
> I would think that tc displays all queues? It says eth0 and eth0 is the
> device that we sent the data out on.
>
> > So based on the current analysis, the packets are getting dropped after
> > the call to ip_local_out() in ip_push_pending_frames(). ip_local_out()
> > is failing with NET_XMIT_DROP. But we are not sure where they are
> > getting dropped. Is that right?
>
> ip_local_out is returning ENOBUFS. Something at the qdisc layer is
> dropping the packet and not incrementing counters.
Is the ENOBUFS return with your/Eric's patch? I thought you were
were seeing NET_XMIT_DROP without any patches.
>
> > I think we need to figure out where they are getting dropped and then
> > decide on the appropriate counter to be incremented.
>
> Right. Where in the qdisc layer do drops occur?
The normal path where the packets are dropped when the tx qlen is exceeded is
pfifo_fast_enqueue() -> qdisc_drop()
In this path, drops are counted.
The other place is in dev_queue_xmit(), but you are not hitting that case too.
So it looks like there is another place where they are getting dropped.
Thanks
Sridhar
^ permalink raw reply
* Re: [PATCH 0/9] drivers/net/s2io.c: Cleanups
From: David Miller @ 2009-08-26 22:33 UTC (permalink / raw)
To: joe
Cc: netdev, anil.murthy, sreenivasa.honnur, sivakumar.subramani,
santosh.rastapur, ram.vepa, akpm, linux-next
In-Reply-To: <cover.1251170438.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Mon, 24 Aug 2009 20:29:39 -0700
> Just a few cleanups, compiled, untested. No hardware.
>
> Joe Perches (9):
> s2io.c: Use const for strings
> s2io.c: Shorten code line length by using intermediate pointers
> s2io.c: Use calculated size in kmallocs
> s2io.c: use kzalloc
> s2io.c: Make more conforming to normal kernel style
> s2io.c: convert printks to pr_<level>
> s2io.c: fix spelling explaination
> s2io.c: Standardize statistics accessors
> s2io.c: Convert skipped nic->config.tx_cfg[i]. to tx_cfg->
Since this is a pretty serious set of cleanups, I'd like to
let the S2IO driver folks have some time to look at this and
at least have a chance to ACK/NACK them.
^ permalink raw reply
* Re: [PATCH NEXT 1/4] netxen: remove unused code
From: David Miller @ 2009-08-26 22:37 UTC (permalink / raw)
To: dhananjay; +Cc: davem, netdev
In-Reply-To: <1251177808-16450-1-git-send-email-dhananjay@netxen.com>
From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Mon, 24 Aug 2009 22:23:25 -0700
> Remove code for phy access on unreleased NX2031 based quad-gig board.
>
> NX3031 based production quad-gig boards do not require direct phy
> access by driver.
>
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Applied.
^ permalink raw reply
* Re: [PATCH NEXT 2/4] netxen: implement generic pcie semaphore functions
From: David Miller @ 2009-08-26 22:37 UTC (permalink / raw)
To: dhananjay; +Cc: davem, netdev
In-Reply-To: <1251177808-16450-2-git-send-email-dhananjay@netxen.com>
From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Mon, 24 Aug 2009 22:23:26 -0700
> Implement common function for locking/unlocking 8 hardware
> semaphores used for serializing access to shared resouces
> on a NIC board by different PCI functions.
>
> As by definition, callers of these semaphore API can be
> put to sleep till the semaphore is locked.
>
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Applied.
^ permalink raw reply
* Re: [PATCH NEXT 3/4] netxen: remove netxen_nic_niu.c
From: David Miller @ 2009-08-26 22:37 UTC (permalink / raw)
To: dhananjay; +Cc: davem, netdev
In-Reply-To: <1251177808-16450-3-git-send-email-dhananjay@netxen.com>
From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Mon, 24 Aug 2009 22:23:27 -0700
> Consolidate all MAC/PHY access functions into netxen_nic_hw.c
>
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Applied.
^ permalink raw reply
* Re: [PATCH NEXT 4/4] netxen: bridged mode optimizations
From: David Miller @ 2009-08-26 22:37 UTC (permalink / raw)
To: dhananjay; +Cc: davem, netdev, narender.kumar
In-Reply-To: <1251177808-16450-4-git-send-email-dhananjay@netxen.com>
From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Mon, 24 Aug 2009 22:23:28 -0700
> From: Narender Kumar <narender.kumar@qlogic.com>
>
> When the interface is put in bridged mode, destination mac
> addresses are unknown to firmware. So packets take a slow
> path (lower priority) in firmware reducing performance.
>
> Firmware can cache limited number of remote unicast mac
> addresses for certain interval, if "dynamic mac learning"
> mode is enabled.
>
> Driver needs to enable this "mac learning" mode in firmware.
> Currently this is done through net device class sysfs entry,
> possibly this can also be done upon netlink notifications to
> from bridge.
>
> Signed-off-by: Narender Kumar <narender.kumar@qlogic.com>
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Applied.
^ permalink raw reply
* Re: [net-next PATCH] e1000: Remove unused function e1000_mta_set.
From: David Miller @ 2009-08-26 22:38 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, david.graham
In-Reply-To: <20090825144250.15297.79619.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 25 Aug 2009 07:43:17 -0700
> From: Graham, David <david.graham@intel.com>
>
> Remove function e1000_mta_set, as it is no longer called
>
> Signed-off-by: Dave Graham <david.graham@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net-next PATCH 1/3] ixgbe: Fix isues while reporting 8259x backplane link capabilities
From: David Miller @ 2009-08-26 22:38 UTC (permalink / raw)
To: jeffrey.t.kirsher
Cc: netdev, gospo, mallikarjuna.chilakala, peter.p.waskiewicz.jr
In-Reply-To: <20090825144709.15560.28811.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 25 Aug 2009 07:47:11 -0700
> From: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
>
> Fix ethtool get_settings logic to report 10G & 1G advertised and
> supported link modes in all 8259x 10G backplane connection types
> except for 82598EB BX network connection type.
>
> Signed-off-by: Mallikarjuna R Chilakala <mallikarjuna.chilakala@intel.com>
> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net-next PATCH 2/3] ixgbe: cleanup functions that should have been defined static
From: David Miller @ 2009-08-26 22:38 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, donald.c.skidmore
In-Reply-To: <20090825144731.15560.73592.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 25 Aug 2009 07:47:32 -0700
> From: Don Skidmore <donald.c.skidmore@intel.com>
>
> We have some ~40 functions that were being called out with 'make
> namespacecheck'. This patch changes these functions to be static.
>
> Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net-next PATCH 3/3] igb/ixgbe: add IPV6_CSUM support to vlan_features
From: David Miller @ 2009-08-26 22:38 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, alexander.h.duyck
In-Reply-To: <20090825144750.15560.4450.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 25 Aug 2009 07:47:50 -0700
> From: Alexander Duyck <alexander.h.duyck@intel.com>
>
> We were already exporting TSO6 to the vlan, but we weren't exporting the
> checksum support for IPV6 which was causing warning messages to be
> displayed when doing IPv6 TSO over a vlan.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-26 22:39 UTC (permalink / raw)
To: Ingo Molnar
Cc: David Miller, rostedt, fweisbec, billfink, netdev, brice,
gallatin
In-Reply-To: <20090826204027.GA21159@elte.hu>
On Wed, Aug 26, 2009 at 10:40:27PM +0200, Ingo Molnar wrote:
>
> * Neil Horman <nhorman@tuxdriver.com> wrote:
>
> > On Wed, Aug 26, 2009 at 09:48:35PM +0200, Ingo Molnar wrote:
> > >
> > > * David Miller <davem@davemloft.net> wrote:
> > >
> > > > From: Ingo Molnar <mingo@elte.hu>
> > > > Date: Wed, 26 Aug 2009 21:08:30 +0200
> > > >
> > > > > Sigh, no. Please re-read the past discussions about this.
> > > > > trace_skb_sources.c is a hack and should be converted to generic
> > > > > tracepoints. Is there anything in it that cannot be expressed in
> > > > > terms of TRACE_EVENT()?
> > > >
> > > > Neil explained why he needed to implement it this way in his reply
> > > > to Steven Rostedt. I attach it here for your convenience.
> > >
> > > thanks. The argument is invalid:
> >
> > Just because you assert that doesn't make it so, Ingo.
>
> I stand by that statement, the argument is invalid, for the many
> reasons i outlined in my previous mails. (you'd have gotten those
> same arguments had you submitted that patch to the folks who
> maintain kernel/trace/)
>
Steven specifically told me to submit the patch to the subsystem maintainer that
I'm adding tracepoints for, and the only feedback I got on it was his one
question, the answer to which I assume satisfied him, due to that there was no
subseuqent discussion. I'm going to ignore your previous emails, because,
despite the various advantages of just using plain TRACE_EVENTs because you
provide the ftrace interface, and I found it useful. Your observation is
correct, I like it, and thats what I wanted to use, so I used it. If you don't
want people to use it, don't provide it.
> > > > > BTW, why not just do this as events? Or was this just a easy way
> > > > > to communicate with the user space tools?
> > > >
> > > > Thats exactly why I did it. the idea is for me to now write a
> > > > user space tool that lets me analyze the events and ajust process
> > > > scheduling to optimize the rx path. Neil
> > >
> > > All tooling (in fact _more_ tooling) can be done based on generic,
> > > TRACE_EVENT() based tracepoints. Generic tracepoints are far more
> > > available, have a generalized format with format parsers and user
> > > tooling implemented, etc. etc.
> >
> > Then why allow for ftrace modules at all? [...]
>
> We routinely reject trivial plugins like yours and ask people to use
> the proper mechanism: TRACE_EVENT().
>
Again, if you consider there to only be one proper mechanism here, don't provide
others.
> We are also converting non-trivial plugins to generic tracepoints. A
> recent example are the system call tracepoints, but we also
> converted blktrace and kmemtrace to generic tracepoints.
>
If you're getting rid of ftrace, then fine, just say so. If the interface I
chose is getting removed, I'll change it. But I'm not going to change it just
because you're going around saying my previous work sucks. Theres nothing wrong
with it, it works quite well right now as it is.
> But trace_skb_sources.c got committed to the networking tree,
> without review and acks from the tracing folks. Now you are
> unwilling to fix it and that's not very constructive.
>
I'm not willing to fix it because its not broken. I submitted it where steven
suggested that I submitted it, and the reviews that I got were positive. All
you've told me is that you think theres a better way. Its fine if theres a
better way, but the way I have currently is sufficient. I have acutal bugs to
fix. Rewriting this to suit your opinions after the fact really isn't
productive for me.
> > [...] I grant that the skb ftracer is a bit trivial at the moment
> > for an ftrace module, but I really prefer to leave it is so that I
> > can expand it with additional tracepoints. And looking at them,
> > anything you've said above applies to any of the currently
> > implemented ftrace modules. If you're so adamant that we should
> > just do everything with TRACE_EVENT log messages, then lets get
> > rid of the ftrace infrastructure all together. Until we do that,
> > however, I like my skb tracer just as it is.
>
> You dont seem to be aware of the breath of features and capabilities
> that TRACE_EVENT() based tooling allows us to do. Please see my
> previous mail about an (incomplete) list.
>
Fine, I grant you that TRACE_EVENT might provide great advantages over an ftrace
module. What you seem to be missing is that an ftrace module is sufficnet for
the needs of what I was tracing.
Ok, I'm rather tired of arguing. Dave, I'll leave this in your hands. The code
I wrote works fairly well in my view, and I feel like the review on it was both
positive and sufficent for inclusion. But thats not my call, its yours. I can
meet my own need with a raw TRACE_EVENT for now just as easily. IF you feel
like the skb plugin should be pulled, please do so, and let me know. All I ask
is that you keep the skb_copy_datagram_iovec TRACE_EVENT in place. If you pull
the ftrace plugin, I'll submit a subsequent patch to agument the printing format
so that I can gather the numa allocation and consumption data directly there.
Regards
Neil
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: David Miller @ 2009-08-26 22:44 UTC (permalink / raw)
To: nhorman; +Cc: mingo, rostedt, fweisbec, billfink, netdev, brice, gallatin
In-Reply-To: <20090826223922.GA25318@hmsreliant.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed, 26 Aug 2009 18:39:22 -0400
> Ok, I'm rather tired of arguing. Dave, I'll leave this in your hands.
I've gotten this kind of urging, both in private and in public, from
both of you now. And I'm sorry, that's not how this works.
It is not my job to somehow force you turkeys how to work effectively
together. :-)
What we can do is ask Mr. Rostedt to asses the situation and give
his feedback.
So if Steven could give some feedback about this specific situation
that would be great and might help us move forward.
^ permalink raw reply
* Re: [PATCH 0/9] drivers/net/s2io.c: Cleanups
From: Joe Perches @ 2009-08-26 22:47 UTC (permalink / raw)
To: David Miller
Cc: netdev, anil.murthy, sreenivasa.honnur, sivakumar.subramani,
santosh.rastapur, ram.vepa, akpm, linux-next
In-Reply-To: <20090826.153353.259743590.davem@davemloft.net>
On Wed, 2009-08-26 at 15:33 -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Mon, 24 Aug 2009 20:29:39 -0700
> > Just a few cleanups, compiled, untested. No hardware.
> > s2io.c: Use const for strings
> > s2io.c: Shorten code line length by using intermediate pointers
> > s2io.c: Use calculated size in kmallocs
> > s2io.c: use kzalloc
> > s2io.c: Make more conforming to normal kernel style
> > s2io.c: convert printks to pr_<level>
> > s2io.c: fix spelling explaination
> > s2io.c: Standardize statistics accessors
> > s2io.c: Convert skipped nic->config.tx_cfg[i]. to tx_cfg->
>
> Since this is a pretty serious set of cleanups, I'd like to
> let the S2IO driver folks have some time to look at this and
> at least have a chance to ACK/NACK them.
No worries. That's fine. It's only cleanups.
I haven't heard from them though and they haven't
submitted or signed a patch for s2io in over a year.
Hey! Neterion people! Is anybody home?
^ permalink raw reply
* Re: [PATCH 00/12] tg3: Updates and bugfixes
From: David Miller @ 2009-08-26 22:52 UTC (permalink / raw)
To: mcarlson; +Cc: netdev, andy
In-Reply-To: <1251232429.25397@xw6200>
From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Tue, 25 Aug 2009 13:06:01 -0700
> This patchset contains a set of bugfixes for the tg3 driver.
>
> Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> Reviewed-by: Michael Chan <mchan@broadcom.com>
> Reviewed-by: Benjamin Li <benli@broadcom.com>
One of the nicest driver patch series submissions I've
seen in a while.
The changes were broken up. And also they were well formed and with
adequately detailed commit messages.
And it even looks compile tested :-)
Nice work, applied to net-next-2.6
Thanks!
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Ingo Molnar @ 2009-08-26 22:57 UTC (permalink / raw)
To: Neil Horman
Cc: David S. Miller, Steven Rostedt, Fr?d?ric Weisbecker, Bill Fink,
Linux Network Developers, brice, gallatin
In-Reply-To: <20090826200119.GD10816@hmsreliant.think-freely.org>
* Neil Horman <nhorman@tuxdriver.com> wrote:
> > Is there anything in it that cannot be expressed in terms of
> > TRACE_EVENT()?
>
> As David noted in my previous posting, no, I don't intend to
> change this. [...]
Well, this change lacks the ack of the maintainers of kernel/trace/*
for the technical reasons outlined in the (many...) mails sent on
this topic, so for the .32 networking tree to be properly pushable
to Linus you'll have to come up with a better answer than "I don't
intend to change this".
David, i tried to help but i really dont have time to deal with an
inefficient workflow like this. Two weeks ago you committed a
clearly broken patch to the tracing code, it had bugs, it was
objected to because it does the wrong thing altogether and Neil
refuses to fix it and i'm supposed to convince Neil what the right
solution is?
That's not how maintenance is supposed to work, it's utterly not
scalable. Please deal with it one way or another.
Thanks,
Ingo
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Steven Rostedt @ 2009-08-26 23:05 UTC (permalink / raw)
To: David Miller
Cc: nhorman, Ingo Molnar, Frederic Weisbecker, billfink, netdev,
brice, gallatin
In-Reply-To: <20090826.154410.95005620.davem@davemloft.net>
On Wed, 26 Aug 2009, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Wed, 26 Aug 2009 18:39:22 -0400
>
> > Ok, I'm rather tired of arguing. Dave, I'll leave this in your hands.
>
> I've gotten this kind of urging, both in private and in public, from
> both of you now. And I'm sorry, that's not how this works.
>
> It is not my job to somehow force you turkeys how to work effectively
> together. :-)
>
> What we can do is ask Mr. Rostedt to asses the situation and give
> his feedback.
>
> So if Steven could give some feedback about this specific situation
> that would be great and might help us move forward.
OK, here's my thought on the matter.
How about Neil try out doing all he can with the existing TRACE_EVENT
work. I'm sure Ingo and myself would be fine with helping him with any
issues he comes up with. If there is something that he hates about it,
that really makes his user space code messy, then he can put the ball back
in our court, and Ingo and I will need to come up with a solution.
If we truly hit a show stopper, than we can always fall back to the ftrace
plugin. But until we find out for sure that TRACE_EVENT is not good
enough, then we should try that out.
How's that sound?
-- Steve
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Ingo Molnar @ 2009-08-26 23:05 UTC (permalink / raw)
To: David Miller
Cc: nhorman, rostedt, fweisbec, billfink, netdev, brice, gallatin
In-Reply-To: <20090826.154410.95005620.davem@davemloft.net>
* David Miller <davem@davemloft.net> wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Wed, 26 Aug 2009 18:39:22 -0400
>
> > Ok, I'm rather tired of arguing. Dave, I'll leave this in your
> > hands.
>
> I've gotten this kind of urging, both in private and in public,
> from both of you now. And I'm sorry, that's not how this works.
>
> It is not my job to somehow force you turkeys how to work
> effectively together. :-)
It is definitely your job to ensure that you do not commit deficient
patches to kernel/trace/ via the networking tree. You created this
situation to begin with so you might as well take some
responsibility and help resolve it.
And the thing is, we are not rigid about these things in the tracing
tree and if this was a good change we would not mind and you'd have
my Ack. The problem is that it's a crappy change and that Neil is
refusing to fix it. So please fix it,
Thanks,
Ingo
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: David Miller @ 2009-08-26 23:08 UTC (permalink / raw)
To: mingo; +Cc: nhorman, rostedt, fweisbec, billfink, netdev, brice, gallatin
In-Reply-To: <20090826230514.GB31970@elte.hu>
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 27 Aug 2009 01:05:14 +0200
> The problem is that it's a crappy change and that Neil is
> refusing to fix it. So please fix it,
Thankfully, Steven Rostedt gave a much more useful and
reasonable response than you.
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: David Miller @ 2009-08-26 23:09 UTC (permalink / raw)
To: rostedt; +Cc: nhorman, mingo, fweisbec, billfink, netdev, brice, gallatin
In-Reply-To: <alpine.DEB.2.00.0908261901270.11291@gandalf.stny.rr.com>
From: Steven Rostedt <rostedt@goodmis.org>
Date: Wed, 26 Aug 2009 19:05:20 -0400 (EDT)
> How about Neil try out doing all he can with the existing TRACE_EVENT
> work. I'm sure Ingo and myself would be fine with helping him with any
> issues he comes up with. If there is something that he hates about it,
> that really makes his user space code messy, then he can put the ball back
> in our court, and Ingo and I will need to come up with a solution.
>
> If we truly hit a show stopper, than we can always fall back to the ftrace
> plugin. But until we find out for sure that TRACE_EVENT is not good
> enough, then we should try that out.
>
> How's that sound?
That works for me, thanks Steven!
I'll revert Neil's change from net-next-2.6 and we can work on a
usable solution, long-term.
^ permalink raw reply
* [PATCH][net-next] LRO: improve aggregation in case of zero TSecr packets
From: Octavian Purdila @ 2009-08-26 23:08 UTC (permalink / raw)
To: Jan-Bernd Themann; +Cc: Christoph Raisch, Eric Dumazet, netdev
[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]
This fixes a temporary performance issue we noticed in back to back
TSO - LRO tests when such tests are run within five minutes after
boot.
The TSval field of TCP packets is filled in based on the current
jiffie, which is initialized at -300*HZ. That means that in 5 minutes
after reboot it will wrap to zero.
While the jiffie value is 0 on the LRO side, TCP packets will be
send out with TSVal zero as well. The TSO side will respond with
packets in which TSecr is set to zero.
At this point LRO will avoid aggregating the packets, suddenly
putting a lot of pressure on the stack which will result in drops and
retransmission for a short while.
There are cases where aggregating zero TSecr is better and cases in
which is not:
1. non zero TSecr packets, zero TSecr packet
Better not to aggregate, otherwise we miss a valid TSecr.
2. zero TSecr packets (amplified by TSO)
Better to aggregate.
3. zero TSecr packet, non zero TSecr packets
OK to aggregate.
4. non zero TSecr packets, zero TSecr packets, non zero TSecr packet
OK to aggregate, but not a big overhead if we aggregate in 2
segments instead of one.
This patch allows aggregation for cases 2 and 3 as well as aggregation
in 2 segments for case 4, while denying aggregation in case 1.
Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
net/ipv4/inet_lro.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
[-- Attachment #2: d943fa4bd69b5ff21505eb6187120fe60868b5f9.diff --]
[-- Type: text/x-patch, Size: 474 bytes --]
diff --git a/net/ipv4/inet_lro.c b/net/ipv4/inet_lro.c
index 6a667da..d16c9d5 100644
--- a/net/ipv4/inet_lro.c
+++ b/net/ipv4/inet_lro.c
@@ -90,9 +90,9 @@ static int lro_tcp_ip_check(struct iphdr *iph, struct tcphdr *tcph,
ntohl(*topt)))
return -1;
- /* timestamp reply should not be zero */
+ /* aggregate if we are sure we won't miss a valid TSecr */
topt++;
- if (*topt == 0)
+ if (*topt == 0 && lro_desc->tcp_rcv_tsecr != 0)
return -1;
}
^ permalink raw reply related
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Ingo Molnar @ 2009-08-26 23:14 UTC (permalink / raw)
To: Neil Horman
Cc: David Miller, rostedt, fweisbec, billfink, netdev, brice,
gallatin
In-Reply-To: <20090826223922.GA25318@hmsreliant.think-freely.org>
* Neil Horman <nhorman@tuxdriver.com> wrote:
> On Wed, Aug 26, 2009 at 10:40:27PM +0200, Ingo Molnar wrote:
> >
> > * Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > > On Wed, Aug 26, 2009 at 09:48:35PM +0200, Ingo Molnar wrote:
> > > >
> > > > * David Miller <davem@davemloft.net> wrote:
> > > >
> > > > > From: Ingo Molnar <mingo@elte.hu>
> > > > > Date: Wed, 26 Aug 2009 21:08:30 +0200
> > > > >
> > > > > > Sigh, no. Please re-read the past discussions about this.
> > > > > > trace_skb_sources.c is a hack and should be converted to generic
> > > > > > tracepoints. Is there anything in it that cannot be expressed in
> > > > > > terms of TRACE_EVENT()?
> > > > >
> > > > > Neil explained why he needed to implement it this way in his reply
> > > > > to Steven Rostedt. I attach it here for your convenience.
> > > >
> > > > thanks. The argument is invalid:
> > >
> > > Just because you assert that doesn't make it so, Ingo.
> >
> > I stand by that statement, the argument is invalid, for the many
> > reasons i outlined in my previous mails. (you'd have gotten those
> > same arguments had you submitted that patch to the folks who
> > maintain kernel/trace/)
>
> Steven specifically told me to submit the patch to the subsystem
> maintainer that I'm adding tracepoints for, and the only feedback
> I got on it was his one question, the answer to which I assume
> satisfied him, due to that there was no subseuqent discussion.
I dont speak for Steve but i cannot imagine him suggesting to you to
add a new plugin to kernel/trace/.
'adding tracepoints' is a shortcut for TRACE_EVENT() these days.
Those are fundamentally decentralized indeed - but that's not what
you used.
> I'm going to ignore your previous emails, because, despite the
> various advantages of just using plain TRACE_EVENTs because you
> provide the ftrace interface, and I found it useful. Your
> observation is correct, I like it, and thats what I wanted to use,
> so I used it. If you don't want people to use it, don't provide
> it.
This might be convenient to you, but that's not how kernel
maintenance works.
By your argument it would be fine for me to add a new networking
protocol to net/ and ignore the objections from networking
maintainers, with the argument that 'you provided protocol
interfaces and i just made use of it and like it'?
> > > > > > BTW, why not just do this as events? Or was this just a easy way
> > > > > > to communicate with the user space tools?
> > > > >
> > > > > Thats exactly why I did it. the idea is for me to now write a
> > > > > user space tool that lets me analyze the events and ajust process
> > > > > scheduling to optimize the rx path. Neil
> > > >
> > > > All tooling (in fact _more_ tooling) can be done based on generic,
> > > > TRACE_EVENT() based tracepoints. Generic tracepoints are far more
> > > > available, have a generalized format with format parsers and user
> > > > tooling implemented, etc. etc.
> > >
> > > Then why allow for ftrace modules at all? [...]
> >
> > We routinely reject trivial plugins like yours and ask people to
> > use the proper mechanism: TRACE_EVENT().
>
> Again, if you consider there to only be one proper mechanism here,
> don't provide others.
We dont provide them. kernel/trace/ is an internal directory to the
tracing subsystem.
> > We are also converting non-trivial plugins to generic tracepoints. A
> > recent example are the system call tracepoints, but we also
> > converted blktrace and kmemtrace to generic tracepoints.
>
> If you're getting rid of ftrace, then fine, just say so. If the
> interface I chose is getting removed, I'll change it. But I'm not
> going to change it just because you're going around saying my
> previous work sucks. Theres nothing wrong with it, it works quite
> well right now as it is.
where did i say that we are getting rid of ftrace? We are not
getting rid of it.
> > But trace_skb_sources.c got committed to the networking tree,
> > without review and acks from the tracing folks. Now you are
> > unwilling to fix it and that's not very constructive.
>
> I'm not willing to fix it because its not broken. I submitted it
> where steven suggested that I submitted it, and the reviews that I
> got were positive. All you've told me is that you think theres a
> better way. Its fine if theres a better way, but the way I have
> currently is sufficient. I have acutal bugs to fix. Rewriting
> this to suit your opinions after the fact really isn't productive
> for me.
No, you should do it differently because 1) it's in the wrong tree
2) the maintainers of this code asked you to do that.
We'd never have committed your patch to the tracing tree - it was
David's mistake to commit it.
Btw., commit 9ec04da74 lacks Steve's ack and has an ugly diffstat
mixed into the commit log:
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Makefile | 1
trace.h | 19 ++++++
trace_skb_sources.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 174 insertions(+)
Signed-off-by: David S. Miller <davem@davemloft.net>
> > > [...] I grant that the skb ftracer is a bit trivial at the moment
> > > for an ftrace module, but I really prefer to leave it is so that I
> > > can expand it with additional tracepoints. And looking at them,
> > > anything you've said above applies to any of the currently
> > > implemented ftrace modules. If you're so adamant that we should
> > > just do everything with TRACE_EVENT log messages, then lets get
> > > rid of the ftrace infrastructure all together. Until we do that,
> > > however, I like my skb tracer just as it is.
> >
> > You dont seem to be aware of the breath of features and capabilities
> > that TRACE_EVENT() based tooling allows us to do. Please see my
> > previous mail about an (incomplete) list.
>
> Fine, I grant you that TRACE_EVENT might provide great advantages
> over an ftrace module. What you seem to be missing is that an
> ftrace module is sufficnet for the needs of what I was tracing.
>
> Ok, I'm rather tired of arguing. Dave, I'll leave this in your
> hands. The code I wrote works fairly well in my view, and I feel
> like the review on it was both positive and sufficent for
> inclusion. But thats not my call, its yours. I can meet my own
> need with a raw TRACE_EVENT for now just as easily. IF you feel
> like the skb plugin should be pulled, please do so, and let me
> know. All I ask is that you keep the skb_copy_datagram_iovec
> TRACE_EVENT in place. If you pull the ftrace plugin, I'll submit
> a subsequent patch to agument the printing format so that I can
> gather the numa allocation and consumption data directly there.
Well, David is not maintaining kernel/trace/ last i checked, so i'm
puzzled why you leave it 'in the hands' of him.
Ingo
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-26 23:19 UTC (permalink / raw)
To: David Miller; +Cc: mingo, rostedt, fweisbec, billfink, netdev, brice, gallatin
In-Reply-To: <20090826.154410.95005620.davem@davemloft.net>
On Wed, Aug 26, 2009 at 03:44:10PM -0700, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Wed, 26 Aug 2009 18:39:22 -0400
>
> > Ok, I'm rather tired of arguing. Dave, I'll leave this in your hands.
>
> I've gotten this kind of urging, both in private and in public, from
> both of you now. And I'm sorry, that's not how this works.
>
> It is not my job to somehow force you turkeys how to work effectively
> together. :-)
>
> What we can do is ask Mr. Rostedt to asses the situation and give
> his feedback.
>
> So if Steven could give some feedback about this specific situation
> that would be great and might help us move forward.
>
Thats a fine solution by me. I'll go with Stevens decision.
Neil
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Neil Horman @ 2009-08-26 23:23 UTC (permalink / raw)
To: Steven Rostedt
Cc: David Miller, Ingo Molnar, Frederic Weisbecker, billfink, netdev,
brice, gallatin
In-Reply-To: <alpine.DEB.2.00.0908261901270.11291@gandalf.stny.rr.com>
On Wed, Aug 26, 2009 at 07:05:20PM -0400, Steven Rostedt wrote:
>
> On Wed, 26 Aug 2009, David Miller wrote:
>
> > From: Neil Horman <nhorman@tuxdriver.com>
> > Date: Wed, 26 Aug 2009 18:39:22 -0400
> >
> > > Ok, I'm rather tired of arguing. Dave, I'll leave this in your hands.
> >
> > I've gotten this kind of urging, both in private and in public, from
> > both of you now. And I'm sorry, that's not how this works.
> >
> > It is not my job to somehow force you turkeys how to work effectively
> > together. :-)
> >
> > What we can do is ask Mr. Rostedt to asses the situation and give
> > his feedback.
> >
> > So if Steven could give some feedback about this specific situation
> > that would be great and might help us move forward.
>
> OK, here's my thought on the matter.
>
> How about Neil try out doing all he can with the existing TRACE_EVENT
> work. I'm sure Ingo and myself would be fine with helping him with any
> issues he comes up with. If there is something that he hates about it,
> that really makes his user space code messy, then he can put the ball back
> in our court, and Ingo and I will need to come up with a solution.
>
> If we truly hit a show stopper, than we can always fall back to the ftrace
> plugin. But until we find out for sure that TRACE_EVENT is not good
> enough, then we should try that out.
>
> How's that sound?
>
Ok, thats fine by me. I really don't have any oposition to just using raw
TRACE_EVENTS for my current purposes, but as Ingo's previous mail shows, theres
_alot_ to it. Using the ftrace interface was really, in the end, just simpler
for me. But if just using TRACE_EVENT is the way it needs to be, so be it.
Dave, would you please revert commit 9ec04da7489d2c9ae01ea6e9b5fa313ccf3d35fb
and 5a165657bef7c47e5ff4cd138f7758ef6278e87b? That should remove the ftrace
code, and leave the TRACE_EVENT tracepoint for skb_copy_datagram_to_iovec in
place. I'll submit a patch in the next few days to augment the TRACE_EVENT
format to export all the data that I need.
Thanks!
Neil
> -- Steve
>
>
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: David Miller @ 2009-08-26 23:29 UTC (permalink / raw)
To: nhorman; +Cc: rostedt, mingo, fweisbec, billfink, netdev, brice, gallatin
In-Reply-To: <20090826232304.GB25980@hmsreliant.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed, 26 Aug 2009 19:23:04 -0400
> Dave, would you please revert commit 9ec04da7489d2c9ae01ea6e9b5fa313ccf3d35fb
> and 5a165657bef7c47e5ff4cd138f7758ef6278e87b? That should remove the ftrace
> code, and leave the TRACE_EVENT tracepoint for skb_copy_datagram_to_iovec in
> place. I'll submit a patch in the next few days to augment the TRACE_EVENT
> format to export all the data that I need.
Ok.
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Ingo Molnar @ 2009-08-26 23:30 UTC (permalink / raw)
To: David Miller
Cc: rostedt, nhorman, fweisbec, billfink, netdev, brice, gallatin
In-Reply-To: <20090826.160910.187803108.davem@davemloft.net>
* David Miller <davem@davemloft.net> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> Date: Wed, 26 Aug 2009 19:05:20 -0400 (EDT)
>
> > How about Neil try out doing all he can with the existing TRACE_EVENT
> > work. I'm sure Ingo and myself would be fine with helping him with any
> > issues he comes up with. If there is something that he hates about it,
> > that really makes his user space code messy, then he can put the ball back
> > in our court, and Ingo and I will need to come up with a solution.
> >
> > If we truly hit a show stopper, than we can always fall back to the ftrace
> > plugin. But until we find out for sure that TRACE_EVENT is not good
> > enough, then we should try that out.
> >
> > How's that sound?
>
> That works for me, thanks Steven!
>
> I'll revert Neil's change from net-next-2.6 and we can work on a
> usable solution, long-term.
thanks David!
Also, my prior offer to help out with the TRACE_EVENT conversion
stands as well, plus more TRACE_EVENT() tracepoints would be welcome
too in the networking code.
It's a very useful feature and they are a lot easier (and more
decentralized) to add than new tracing plugins.
Thanks,
Ingo
^ 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