On 2012-05-31 09:38, Paolo Bonzini wrote:
> Il 31/05/2012 00:53, Luigi Rizzo ha scritto:
>> The image contains my fast packet generator "pkt-gen" (a stock
>> traffic generator such as netperf etc. is too slow to show the
>> problem). pkt-gen can send about 1Mpps in this configuration using
>> -net netmap in the backend. The qemu process in this case takes 100%
>> CPU. On the receive side, i cannot receive more than 50Kpps, even if i
>> flood the bridge with a a huge amount of traffic. The qemu process stays
>> at 5% cpu or less.
>>
>> Then i read on the docs in main-loop.h which says that one case where
>> the qemu_notify_event() is needed is when using
>> qemu_set_fd_handler2(), which is exactly what my backend uses
>> (similar to tap.c)
>
> The path is a bit involved, but I think Luigi is right. The docs say
> "Remember to call qemu_notify_event whenever the [return value of the
> fd_read_poll callback] may change from false to true." Now net/tap.c has
>
> static int tap_can_send(void *opaque)
> {
> TAPState *s = opaque;
>
> return qemu_can_send_packet(&s->nc);
> }
>
> and (ignoring VLANs) qemu_can_send_packet is
>
> int qemu_can_send_packet(VLANClientState *sender)
> {
> if (sender->peer->receive_disabled) {
> return 0;
> } else if (sender->peer->info->can_receive &&
> !sender->peer->info->can_receive(sender->peer)) {
> return 0;
> } else {
> return 1;
> }
> }
>
> So whenever receive_disabled goes from 0 to 1 or can_receive goes from 0 to 1,
> the _peer_ has to call qemu_notify_event. In e1000.c we have
>
> static bool e1000_has_rxbufs(E1000State *s, size_t total_size)
> {
> int bufs;
> /* Fast-path short packets */
> if (total_size <= s->rxbuf_size) {
> return s->mac_reg[RDH] != s->mac_reg[RDT] || !s->check_rxov;
> }
> if (s->mac_reg[RDH] < s->mac_reg[RDT]) {
> bufs = s->mac_reg[RDT] - s->mac_reg[RDH];
> } else if (s->mac_reg[RDH] > s->mac_reg[RDT] || !s->check_rxov) {
> bufs = s->mac_reg[RDLEN] / sizeof(struct e1000_rx_desc) +
> s->mac_reg[RDT] - s->mac_reg[RDH];
> } else {
> return false;
> }
> return total_size <= bufs * s->rxbuf_size;
> }
>
> static int
> e1000_can_receive(VLANClientState *nc)
> {
> E1000State *s = DO_UPCAST(NICState, nc, nc)->opaque;
>
> return (s->mac_reg[RCTL] & E1000_RCTL_EN) && e1000_has_rxbufs(s, 1);
> }
>
> So as a conservative approximation, you need to fire qemu_notify_event
> whenever you write to RDH, RDT, RDLEN and RCTL, or when check_rxov becomes
> zero. In practice, only RDT, RCTL and check_rxov matter. Luigi, does this
> patch work for you?
>
> diff --git a/hw/e1000.c b/hw/e1000.c
> index 4573f13..0069103 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -295,6 +295,7 @@ set_rx_control(E1000State *s, int index, uint32_t val)
> s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1;
> DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT],
> s->mac_reg[RCTL]);
> + qemu_notify_event();
> }
>
> static void
> @@ -922,6 +923,7 @@ set_rdt(E1000State *s, int index, uint32_t val)
> {
> s->check_rxov = 0;
> s->mac_reg[index] = val & 0xffff;
> + qemu_notify_event();This still looks like the wrong tool: Packets that can't be delivered are queued. So we need to flush the queue and clear the blocked delivery there. qemu_flush_queued_packets appears more appropriate for this. Conceptually, the backend should be responsible for kicking the iothread as needed. Jan
signature.asc
Description: OpenPGP digital signature
