From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53518) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rs2Tq-00022a-BC for qemu-devel@nongnu.org; Mon, 30 Jan 2012 20:25:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rs2Tp-0003jI-0O for qemu-devel@nongnu.org; Mon, 30 Jan 2012 20:25:54 -0500 Received: from serv2.oss.ntt.co.jp ([222.151.198.100]:58230) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rs2To-0003jB-Hr for qemu-devel@nongnu.org; Mon, 30 Jan 2012 20:25:52 -0500 Message-ID: <4F274319.7000301@oss.ntt.co.jp> Date: Tue, 31 Jan 2012 10:25:45 +0900 From: =?UTF-8?B?RmVybmFuZG8gTHVpcyBWw6F6cXVleiBDYW8=?= MIME-Version: 1.0 References: <1327889855.3455.2.camel@nexus.oss.ntt.co.jp> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] rtl8139: honor RxOverflow flag in can_receive method List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Kovalenko Cc: Mark McLoughlin , Anthony Liguori , =?UTF-8?B?6KeS55SwIOWLh+S6ug==?= , qemu-devel@nongnu.org, "Michael S. Tsirkin" Hi Igor, Thank you for the prompt reply. I really appreciate it. (2012=E5=B9=B401=E6=9C=8831=E6=97=A5 02:28), Igor Kovalenko wrote: > 2012/1/30 Fernando Luis V=C3=A1zquez Cao: >> Some drivers (Linux' 8139too among them) rely on the NIC injecting an = interrupt >> in the event of a receive buffer overflow and, accordingly, set the Rx= Overflow >> bit in the interrupt mask. Unfortunately rtl8139's can_receive method = ignores >> the RxOverflow flag, which may lead to a situation where rtl8139 stops= receiving >> packets (can_receive returns 0) when the receive buffer becomes full. >> >> If the driver eventually read from the receive buffer or reset the car= d the >> emulator could recover from this situation. However some implementatio= ns only >> do this upon receiving an interrupt with either RxOK or RxOverflow set= in the >> ISR; interrupt that will never come because QEMU's flow control mechan= isms would >> prevent rtl8139 from receiving any packet. >> >> Letting packets go through when the overflow interrupt is enabled make= s the >> QEMU emulator compliant to the spec and solves the problem. >> >> This patch should fix a relatively common (in our experience) network = stall >> observed when running enterprise distros with rtl8139 as the NIC; in s= ome cases >> the 8139too device driver gets loaded and when under heavy load the ne= twork >> eventually stops working. > It would be great to see specific example to verify the issue. Sure. In the gdb debug session included inline below you can see that=20 after a while RxBufPtr-RxBufAddr becomes too small to accommodate a MTU sized packet and, because of the current can_receive logic, rtl8139 will stop=20 receiving packets from other QEMU VLAN clients. > Otherwise the change looks great. Could I have your "Acked-by"? By the way, whose tree should this patch=20 go through? ----- gdb debug session (gdb) p *(RTL8139State *) 0x144143a0 $3 =3D {phys =3D "TR\000bS:\000", mult =3D "\000\000\000\200\000\000\004@= ",=20 TxStatus =3D {565346, 565346, 565346, 565346}, TxAddr =3D {899350528,=20 899352064, 899353600, 899355136}, RxBuf =3D 894697472, RxBufferSize =3D 32768, RxBufPtr =3D= 0,=20 RxBufAddr =3D 0, IntrStatus =3D 0, IntrMask =3D 49279, TxConfig =3D 20048= 78976,=20 RxConfig =3D 63374, RxMissed =3D 0, CSCR =3D 832, Cfg9346 =3D 0 '\0', Config0 =3D 0 '\0', = Config1=20 =3D 12 '\f', Config3 =3D 1 '\001', Config4 =3D 0 '\0', Config5 =3D 0 '\0'= , clock_enabled =3D 1 '\001', bChipCmdState =3D 12 '\f', MultiIntr =3D 0= ,=20 BasicModeCtrl =3D 4096, BasicModeStatus =3D 30765, NWayAdvert =3D 1505,=20 NWayLPAR =3D 1505, NWayExpansion =3D 1, CpCmd =3D 0, TxThresh =3D 0 '\0', pci_dev =3D=20 0x14414150, vc =3D 0x14414550, macaddr =3D "TR\000bS:", rtl8139_mmio_io_a= ddr=20 =3D 104, currTxDesc =3D 0, cplus_enabled =3D 0, currCPlusRxDesc =3D 0, currCPlusTxDesc =3D 0,=20 RxRingAddrLO =3D 0, RxRingAddrHI =3D 0, eeprom =3D {contents =3D {33065, = 4332,=20 33081, 0, 0, 0, 0, 21076, 25088, 14931, 0 }, mode =3D 1, tick =3D 0, addre= ss=20 =3D 9 '\t', input =3D 0, output =3D 0, eecs =3D 1 '\001', eesk =3D 0 '\0'= , eedi =3D=20 0 '\0', eedo =3D 1 '\001'}, TCTR =3D 0, TimerInt =3D 0, TCTR_base =3D 0,=20 tally_counters =3D {TxOk =3D 0, RxOk =3D 0, TxERR =3D 0, RxERR =3D 10, Mi= ssPkt =3D=20 0, FAE =3D 0, Tx1Col =3D 0, TxMCol =3D 0, RxOkPhy =3D 1302, RxOkBrd =3D 2, RxOkMul =3D 10, TxAbt= =3D 0,=20 TxUndrn =3D 0}, cplus_txbuffer =3D 0x0, cplus_txbuffer_len =3D 0,=20 cplus_txbuffer_offset =3D 0, timer =3D 0x0} (gdb) c Continuing. [Thread 0x42c3a940 (LWP 2739) exited] [New Thread 0x42c3a940 (LWP 2791)] Program received signal SIGINT, Interrupt. 0x0000003a3a4cced2 in select () from /lib64/libc.so.6 (gdb) d Delete all breakpoints? (y or n) y (gdb) (gdb) c Continuing. Program received signal SIGINT, Interrupt. 0x0000003a3a4cced2 in select () from /lib64/libc.so.6 (gdb) p *(RTL8139State *) 0x144143a0 $4 =3D {phys =3D "TR\000bS:\000", mult =3D "\000\000\000\200\000\000\004@= ",=20 TxStatus =3D {565308, 565346, 565346, 565346}, TxAddr =3D {899350528,=20 899352064, 899353600, 899355136}, RxBuf =3D 894697472, RxBufferSize =3D 32768, RxBufPtr =3D= =20 22032, RxBufAddr =3D 20544, IntrStatus =3D 0, IntrMask =3D 49279, TxConfi= g =3D=20 2004878976, RxConfig =3D 63374, RxMissed =3D 0, CSCR =3D 832, Cfg9346 =3D 0 '\0', = Config0=20 =3D 0 '\0', Config1 =3D 12 '\f', Config3 =3D 1 '\001', Config4 =3D 0 '\0'= ,=20 Config5 =3D 0 '\0', clock_enabled =3D 1 '\001', bChipCmdState =3D 12 '\f', MultiIntr =3D 0= ,=20 BasicModeCtrl =3D 4096, BasicModeStatus =3D 30765, NWayAdvert =3D 1505,=20 NWayLPAR =3D 1505, NWayExpansion =3D 1, CpCmd =3D 0, TxThresh =3D 0 '\0', pci_dev =3D=20 0x14414150, vc =3D 0x14414550, macaddr =3D "TR\000bS:", rtl8139_mmio_io_a= ddr=20 =3D 104, currTxDesc =3D 1, cplus_enabled =3D 0, currCPlusRxDesc =3D 0, currCPlusTxDesc =3D 0,=20 RxRingAddrLO =3D 0, RxRingAddrHI =3D 0, eeprom =3D {contents =3D {33065, = 4332,=20 33081, 0, 0, 0, 0, 21076, 25088, 14931, 0 }, mode =3D 1, tick =3D 0, addre= ss=20 =3D 9 '\t', input =3D 0, output =3D 0, eecs =3D 1 '\001', eesk =3D 0 '\0'= , eedi =3D=20 0 '\0', eedo =3D 1 '\001'}, TCTR =3D 0, TimerInt =3D 0, TCTR_base =3D 0,=20 tally_counters =3D {TxOk =3D 0, RxOk =3D 0, TxERR =3D 0, RxERR =3D 10, Mi= ssPkt =3D=20 0, FAE =3D 0, Tx1Col =3D 0, TxMCol =3D 0, RxOkPhy =3D 10277, RxOkBrd =3D 3, RxOkMul =3D 10, TxAb= t =3D 0,=20 TxUndrn =3D 0}, cplus_txbuffer =3D 0x0, cplus_txbuffer_len =3D 0,=20 cplus_txbuffer_offset =3D 0, timer =3D 0x0} ----- Thanks, Fernando