On Mon, Jul 8, 2024 at 1:17 PM Yong Huang <yong.hu...@smartx.com> wrote: > > > > On Mon, Jul 8, 2024 at 11:21 AM Jason Wang <jasow...@redhat.com> wrote: >> >> On Sat, Jul 6, 2024 at 4:30 AM Hyman Huang <yong.hu...@smartx.com> wrote: >> > >> > Unexpected work by certain Windows guests equipped with the e1000 >> > interface can cause the network to go down and never come back up >> > again unless the guest's interface is reset. >> > >> > To reproduce the failure: >> > 1. Set up two guests with a Windows 2016 or 2019 server operating >> > system. >> >> I vaguely remember e1000 support for Windows has been deprecated for >> several years... >> >> That's why e1000e or igb is implemented in Qemu. >> >> > 2. Set up the e1000 interface for the guests. >> > 3. Pressurize the network slightly between two guests using the iPerf tool. >> > >> > The network goes down after a few days (2-5days), and the issue >> > is the result of not adhering to the e1000 specification. Refer >> > to the details of the specification at the following link: >> > https://www.intel.com/content/dam/doc/manual/pci-pci-x-family-gbe-controllers-software-dev-manual.pdf >> > >> > Chapter 3.2.6 describe the Receive Descriptor Tail register(RDT) >> > as following: >> > This register holds a value that is an offset from the base, and >> > identifies the location beyond the last descriptor hardware can >> > process. Note that tail should still point to an area in the >> > descriptor ring (somewhere between RDBA and RDBA + RDLEN). >> > This is because tail points to the location where software writes >> > the first new descriptor. >> > >> > This means that if the provider—in this case, QEMU—has not yet >> > loaded the packet, >> >> What do you mean by "load" here? > > > Sorry for failing to describe the details. > > The guest driver retrieves the packet from the receive ring buffer > after QEMU forwards it from the tun/tap interface in the e1000 > emulation. > > I used "load" to express "putting packets into the receive ring buffer." > >> >> >> > RDT should never point to that place. >> >> And "that place"? > > If a descriptor in the receive ring buffer has not been filled with a > packet address by QEMU, the descriptor therefore doesn't have any > available packets. The location of the descriptor should not be referred > to by RDT because the location is in the range that "hardware" handles. > > "that place" means the location of the descriptor in the ring buffer > that QEMU hasn't set any available packets related to. > >> >> >> > When >> > implementing the emulation of the e1000 interface, QEMU evaluates >> > if the receive ring buffer is full once the RDT equals the RDH, >> > based on the assumption that guest drivers adhere to this >> > criterion strictly. >> > >> > We applied the following log patch to assist in analyzing the >> > issue and eventually obtained the unexpected information. >> > >> > Log patch: >> > ----------------------------------------------------------------- >> > |--- a/hw/net/e1000.c >> > |+++ b/hw/net/e1000.c >> > |@@ -836,6 +836,9 @@ e1000_set_link_status(NetClientState *nc) >> > | static bool e1000_has_rxbufs(E1000State *s, size_t total_size) >> > | { >> > | int bufs; >> > |+ DBGOUT(RX, "rxbuf_size = %u, s->mac_reg[RDLEN] = %u, s->mac_reg[RDH] >> > = %u, s->mac_reg[RDT] = %u\n", >> > |+ s->rxbuf_size, s->mac_reg[RDLEN], s->mac_reg[RDH], >> > s->mac_reg[RDT]); >> > |+ >> > | /* Fast-path short packets */ >> > | if (total_size <= s->rxbuf_size) { >> > | if (s->mac_reg[RDH] == s->mac_reg[RDT] && s->last_overrun) >> > |@@ -1022,6 +1025,9 @@ e1000_receive_iov(NetClientState *nc, const struct >> > iovec *iov, int iovcnt) >> > | s->rxbuf_min_shift) >> > | n |= E1000_ICS_RXDMT0; >> > | >> > |+ DBGOUT(RX, "rxbuf_size = %u, s->mac_reg[RDLEN] = %u, s->mac_reg[RDH] >> > = %u, s->mac_reg[RDT] = %u\n", >> > |+ s->rxbuf_size, s->mac_reg[RDLEN], s->mac_reg[RDH], >> > s->mac_reg[RDT]); >> > |+ >> > ----------------------------------------------------------------- >> > >> > The last few logs of information when the network is down: >> > >> > e1000: total_size = 1, rxbuf_size = 2048, s->mac_reg[RDLEN] = 16384, >> > s->mac_reg[RDH] = 897, s->mac_reg[RDT] = 885 >> > <- the receive ring buffer is checked for fullness in the >> > e1000_has_rxbufs function, not full. >> > >> > e1000: total_size = 64, rxbuf_size = 2048, s->mac_reg[RDLEN] = 16384, >> > s->mac_reg[RDH] = 898, s->mac_reg[RDT] = 885 >> > <- RDT stays the same, RDH updates to 898, and 1 descriptor >> > utilized after putting the packet to ring buffer. >> > >> > e1000: total_size = 1, rxbuf_size = 2048, s->mac_reg[RDLEN] = 16384, >> > s->mac_reg[RDH] = 898, s->mac_reg[RDT] = 885 >> > <- the receive ring buffer is checked for fullness in the >> > e1000_has_rxbufs function, not full. >> > >> > e1000: total_size = 64, rxbuf_size = 2048, s->mac_reg[RDLEN] = 16384, >> > s->mac_reg[RDH] = 899, s->mac_reg[RDT] = 885 >> > <- RDT stays the same, RDH updates to 899, and 1 descriptor >> > utilized after putting the packet to ring buffer. >> > >> > e1000: total_size = 1, rxbuf_size = 2048, s->mac_reg[RDLEN] = 16384, >> > s->mac_reg[RDH] = 899, s->mac_reg[RDT] = 885 >> > <- the receive ring buffer is checked for fullness in the >> > e1000_has_rxbufs function, not full. >> > >> > e1000: total_size = 64, rxbuf_size = 2048, s->mac_reg[RDLEN] = 16384, >> > s->mac_reg[RDH] = 900, s->mac_reg[RDT] = 885 >> > <- RDT stays the same, RDH updates to 900 , and 1 descriptor >> > utilized after putting the packet to ring buffer. >> > >> > e1000: total_size = 1, rxbuf_size = 2048, s->mac_reg[RDLEN] = 16384, >> > s->mac_reg[RDH] = 900, s->mac_reg[RDT] = 900 >> > <- The ring is full, according to e1000_has_rxbufs, because >> > of the RDT update to 900 and equals RDH ! >> >> Just to make sure I understand this, RDT==RDH means the ring is empty I >> think? >> >> >> See commit: >> >> commit e5b8b0d4ba29fe1268ba049519a1b0cf8552a21a >> Author: Dmitry Fleytman <dmi...@daynix.com> >> Date: Fri Oct 19 07:56:55 2012 +0200 >> >> e1000: drop check_rxov, always treat RX ring with RDH == RDT as empty >> >> Real HW always treats RX ring with RDH == RDT as empty. >> Emulation is supposed to behave the same. > > > Indeed, I'm confused :(, the description in the comment claims that RX > rings with RDH == RDT as empty, but in implementation, it treats that as > overrun. > > See the following 2 contexts: > > 1. e1000_can_receive: > static bool e1000_can_receive(NetClientState *nc) > { > E1000State *s = qemu_get_nic_opaque(nc); > // e1000_has_rxbufs return true means ring buffer has > // available descriptors to use for QEMU. > // false means ring buffer overrun and QEMU should queue the packet > // and wait for the RDT update and available descriptors can be used. > > return e1000x_rx_ready(&s->parent_obj, s->mac_reg) && > e1000_has_rxbufs(s, 1) && !timer_pending(s->flush_queue_timer); > }
Well we had in e1000_has_rx_bufs if (total_size <= s->rxbuf_size) { return s->mac_reg[RDH] != s->mac_reg[RDT]; } RDT!=RDH means RX ring has available descriptors for hardware? Adding more people. Thanks