* Re: multicast routing and multiple interfaces with same IP
From: Octavian Purdila @ 2009-08-26 23:53 UTC (permalink / raw)
To: Ilia K.; +Cc: netdev
In-Reply-To: <1b9338490908111626j55b49177q71b8a373b6e6381b@mail.gmail.com>
On Wednesday 12 August 2009 02:26:21 Ilia K. wrote:
> Hi All,
> When routing daemon wants to enable forwarding of multicast traffic it
> performs something like:
>
> struct vifctl vc = {
> .vifc_vifi = 1,
> .vifc_flags = 0,
> .vifc_threshold = 1,
> .vifc_rate_limit = 0,
> .vifc_lcl_addr = ip, /* <--- ip address of physical interface, e.g. eth0
> */ .vifc_rmt_addr.s_addr = htonl(INADDR_ANY),
> };
> setsockopt(fd, IPPROTO_IP, MRT_ADD_VIF, &vc, sizeof(vc));
>
> This leads (in the kernel) to call to vif_add() function call which
> search the (physical) device using assigned IP address:
> dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
>
> It seems like API (struct vifctl) does not allow to specify an
> interface other way than using it's IP, and if there are more than a
> single interface with specified IP only the first one will be found
> (for example it makes problems when tunnel is configured using the
> same IP as underlying interface).
>
> Am I correct in identifying the problem?
> I can propose the attached patch against 2.6.30.4.
>
Hi Ilia,
I don't have context on multicast routing, but this caught my attention:
>@@ -61,11 +61,13 @@
> unsigned int vifc_rate_limit; /* Rate limiter values (NI) */
> struct in_addr vifc_lcl_addr; /* Our address */
> struct in_addr vifc_rmt_addr; /* IPIP tunnel addr */
>+ int ifindex; /* Local interface index */
> };
>
Wouldn't this break userspace ABI?
Perhaps you could use a union between vifc_lcl_addr and vifc_ifindex, they seem
to be exclusive.
tavi
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Frederic Weisbecker @ 2009-08-26 23:46 UTC (permalink / raw)
To: Neil Horman
Cc: Ingo Molnar, David Miller, rostedt, billfink, netdev, brice,
gallatin
In-Reply-To: <20090826202344.GE10816@hmsreliant.think-freely.org>
On Wed, Aug 26, 2009 at 04:23:44PM -0400, Neil Horman 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.
>
> > > > 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?
Well, the old way to implement a tracer was done as you did: create
a whole ftrace plugin (ie: a tracer).
But it's a bit of a burden to implement a tracer: you have to deal
with ring buffer directly using code that is pretty the same from
a trivial tracer to another, you have to deal with output formatting,
define explicitely your fields, their types, their format separately
if you want the filters to be supported.
Oh and you also need to handle your tracepoints by hand, check their
registration results. You also need to implement by your stop and start
callbacks that deactivate your tracepoints.
So that's a lot of repetitive and error-prone work.
Also kernel/trace hosts a lot of such error-prone code and it doesn't
only become a due diligence of maintainance from you but also for us.
The goal of the TRACE_EVENTs is to reduce the impact of everything I explained
above. You only need to care with the strict necessary things for your traces:
- field name
- field type
- field formats
And that's pretty all. All the burden of copying in the ring buffer, filtering,
tracepoints, formats, output is done in background.
Also your tracer becomes non-ABI dependant because the formats of your fields
are dynamically described in dedicated debugfs files.
Tracer fields, even though we have workarounds to describe their format, have
much more contraints. Their format have a bit more constraints to be fixed.
Also a lot of things are developed in userspace that can profit to every TRACE_EVENTs
as Ingo has shown with perf. Steve's trace-cmd tool also handles them.
The ftrace tracers plugin are still used for non trivial cases where tracing
based on tracepoints are not sufficient. For example the function/function graph
tracers that require hot patching and a gcc feature plus a lot of background subtle
things, or the preemptoff/irqsoff/preemptirqsoff tracers that require a snapshot
of a maximum latency trace, etc...
That's why the ftrace tracers plugins still exist: to cover the non-trivial
cases. But using them for tracing based on simple static tracepoints like yours
is a pure legacy.
Frederic.
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Steven Rostedt @ 2009-08-26 23:33 UTC (permalink / raw)
To: Neil Horman
Cc: Ingo Molnar, David Miller, fweisbec, billfink, netdev, brice,
gallatin
In-Reply-To: <20090826223922.GA25318@hmsreliant.think-freely.org>
On Wed, 26 Aug 2009, Neil Horman 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'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.
Actually, I suggested to submit it to the subsystem maintainer if there
was no changes to the tracing infrastructure. We may have just had a
misunderstanding there. No biggy.
Yes, the plugins are there for the tracers that are not really events.
Those are the latency tracers (they have a double trace buffer for
recording maxes), the function tracers (they are a separate beast
themselves). The other tracers are on their way to being obsoleted.
Ideally the only plugins we should have are:
function, function_graph, mmiotrace, wakeup_rt, wakeup, irqsoff,
preemptoff, preemptirqsoff.
The mmiotrace is a neat thing that traps calls of binary drivers to their
devices, and traces what is written and read.
Thus, the plugins are reserved for the off the wall type of tracing. Not
something that can easily be accomplished with tracepoints.
Currently sched_switch is still there, because the recording of
task->comm's is associated with that tracer, and until we remove that
binding, it will stay. But expect it to eventually disappear too.
>
> > > > > > 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.
I guess the issue is that the plugins were there first, and that we did
what trace events do today with the plugins. When TRACE_EVENT became
mature, it obsoleted a lot of the plugins. Thus we are trying to get rid
of them. But for those tracers that do not do events, then we still need
the plugin facility.
>
> > 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.
It does not suck, but it's "old school" ;-)
>
> > 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 feel guilty here. I misunderstood the scope of your changes, and did not
realize you were adding a plugin.
>
> > > [...] 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.
Yes, please keep the TRACE_EVENT (I think we can all agree on that ;-).
You probably already read my previous email on the matter. Don't delete
your plugin patch until we get everything you need with TRACE_EVENT alone.
Thanks,
-- Steve
^ 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
* 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: 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: 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: 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
* [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: 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
* 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: 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: 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 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: [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: [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: 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: 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: [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: [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 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] 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: [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: [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 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
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