Date: Fri, 28 Aug 2015 12:04:32 -0700 From: Jack Vogel <jfvogel@gmail.com> To: John-Mark Gurney <jmg@funkthat.com> Cc: Sean Bruno <sbruno@freebsd.org>, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: Network card interrupt handling Message-ID: <CAFOYbcm55rJ2ZLyp0J8nkMcd2zCsTjS7GcbE1ZOJ7uXH91q7Bg@mail.gmail.com> In-Reply-To: <20150828184800.GE33167@funkthat.com> References: <55DDE9B8.4080903@freebsd.org> <20150828184800.GE33167@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
The reason the extra handling was added into the local timer was due to chasing hangs in the past, the thought was an interrupt may have been missed. Flags sound like a nice idea, but there is the possibility of a race condition and something still gets missed. Its been quite a few years ago, but there was a time when the em driver was having very intermittent hangs, in fact Sean may have been one of the victims, and this stuff was an attempt to solve that. Every time I looked at the em driver it just cried out to be thoroughly cleaned up or rewritten, but regression testing would be a pain doing that too. In any case, its no longer my job, and I'm glad Sean is giving it the attention he is :) Jack On Fri, Aug 28, 2015 at 11:48 AM, John-Mark Gurney <jmg@funkthat.com> wrote: > Sean Bruno wrote this message on Wed, Aug 26, 2015 at 09:30 -0700: > > -----BEGIN PGP SIGNED MESSAGE----- > > Hash: SHA512 > > > > We've been diagnosing what appeared to be out of order processing in > > the network stack this week only to find out that the network card > > driver was shoveling bits to us out of order (em). > > > > This *seems* to be due to a design choice where the driver is allowed > > to assert a "soft interrupt" to the h/w device while real interrupts > > are disabled. This allows a fake "em_msix_rx" to be started *while* > > "em_handle_que" is running from the taskqueue. We've isolated and > > worked around this by setting our processing_limit in the driver to > > - -1. This means that *most* packet processing is now handled in the > > MSI-X handler instead of being deferred. Some periodic interference > > is still detectable via em_local_timer() which causes one of these > > "fake" interrupt assertions in the normal, card is *not* hung case. > > I have a better question, for MSI-X, we have a dedicated interrupt > thread to do the processing, so why are we even doing any moderation > in this case? It's not any different than spinning in the task queue.. > > How about the attached patch that just disables taskqueue processing > for MSI-X RX interrupts, and does all processing in the interrupt > thread? > > Do you need to add the rx_task to the em_local_timer? If so, then > I would look at setting a flag in the _rxeof that says that processing > is happening... and in the case of the taskqueue, when it sees this > flag set, it just exits, while for the interrupt filter case, we'd > need to be more careful (possibly set a flag that the taskqueue will > inspect, and cause it to stop processing the rx queue)... > > > Both functions use identical code for a start. Both end up down > > inside of em_rxeof() to process packets. Both drop the RX lock prior > > to handing the data up the network stack. > > > > This means that the em_handle_que running from the taskqueue will be > > preempted. Dtrace confirms that this allows out of order processing > > to occur at times and generates a lot of resets. > > > > The reason I'm bringing this up on -arch and not on -net is that this > > is a common design pattern in some of the Ethernet drivers. We've > > done preliminary tests on a patch that moves *all* processing of RX > > packets to the rx_task taskqueue, which means that em_handle_que is > > now the only path to get packets processed. > > > > <stable10 diff> > > https://people.freebsd.org/~sbruno/em_interupt_to_taskqueue.diff > > > > My sense is that this is a slightly "better" method to handle the > > packets but removes some immediacy from packet processing since all > > processing is deferred. However, all packet processing is now > > serialized per queue, which I think is the proper implementation. > > > > Am I smoking "le dope" here or is this the way forward? > > I think you discovered an interresting issue.. > > btw, since you're hacking on em a lot, interrested in fixing em's > jumbo frames so it doesn't use 9k clusters, but instead page sized > clusters? > > -- > John-Mark Gurney Voice: +1 415 225 5579 > > "All that I will do, has been done, All that I have, has not." > > _______________________________________________ > freebsd-arch@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFOYbcm55rJ2ZLyp0J8nkMcd2zCsTjS7GcbE1ZOJ7uXH91q7Bg>