On Fri, Aug 21, 2015 at 02:59:25PM -0700, Vladislav Yasevich wrote:
> In standard operation mode, when the receive ring buffer
> is full, the buffer actually appears empty to the driver since
> the RxBufAddr (the location we wirte new data to) and RxBufPtr
> (the location guest would stat reading from) are the same.
> As a result, the call to rtl8139_RxBufferEmpty ends up
> returning true indicating that the receive buffer is empty.
> This would result in the next packet overwriting the recevie buffer
> again and stalling receive operations.
>
> This patch catches the "receive buffer full" condition
> using an unused C+ register. This is done to simplify
> migration and not require a new machine type.
>
> Signed-off-by: Vladislav Yasevich <vyase...@redhat.com>
> ---
> hw/net/rtl8139.c | 36 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
The rtl8139 code duplicates the following expression in several places:
MOD2(s->RxBufferSize + s->RxBufAddr - s->RxBufPtr, s->RxBufferSize);
It may be cleaner to keep a rx_unread_bytes counter so that all these
users can simply look at that variable.
That cleanup also eliminates the rx full vs empty problem because then
we'll know whether rx_unread_bytes == 0 or rx_unread_bytes ==
s->RxBufferSize.
The same trick of stashing the value in s->currCPlusRxDesc could be
used.
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> index 359e001..3d572ab 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -816,6 +816,23 @@ static int rtl8139_can_receive(NetClientState *nc)
> }
> }
>
> +static void rtl8139_set_rxbuf_full(RTL8139State *s, bool full)
> +{
> + /* In standard mode, C+ RxDesc isn't used. Reuse it
> + * to store the rx_buf_full status.
> + */
assert(!s->cplus_enabled)?
> + s->currCPlusRxDesc = full;
> + DPRINTF("received: rx buffer full\n");
> +}
> +
> +static bool rtl8139_rxbuf_full(RTL8139State *s)
> +{
> + /* In standard mode, C+ RxDesc isn't used. Reuse it
> + * to store the rx_buf_full status.
> + */
assert(!s->cplus_enabled)?
> @@ -2601,6 +2630,9 @@ static void rtl8139_RxBufPtr_write(RTL8139State *s,
> uint32_t val)
> /* this value is off by 16 */
> s->RxBufPtr = MOD2(val + 0x10, s->RxBufferSize);
>
> + /* We just read data, clear full buffer state */
> + rtl8139_set_rxbuf_full(s, false);
> +
> /* more buffer space may be available so try to receive */
> qemu_flush_queued_packets(qemu_get_queue(s->nic));
What if the guest writes this register while we're in C+ mode?
