Re: [Qemu-devel] [RFC Patch v2 05/10] virtio-net rsc: Create timer to drain the packets from the cache pool

[Wei Xu <wexu@redhat.com>]

On 02/01/2016 02:28 PM, Jason Wang wrote:
>
> On 02/01/2016 02:13 AM, wexu@redhat.com wrote:
>> From: Wei Xu <wei@wei-thinkpad.nay.redhat.com>
>>
>> The timer will only be triggered if the packets pool is not empty,
>> and it'll drain off all the cached packets, this is to reduce the
>> delay to upper layer protocol stack.
>>
>> Signed-off-by: Wei Xu <wexu@redhat.com>
>> ---
>>   hw/net/virtio-net.c | 38 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 38 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 4f77fbe..93df0d5 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -48,12 +48,17 @@
>>   
>>   #define MAX_VIRTIO_IP_PAYLOAD  (65535 + IP_OFFSET)
>>   
>> +/* Purge coalesced packets timer interval */
>> +#define RSC_TIMER_INTERVAL  500000
> Any hints for choosing this as default value? Do we need a property for
> user to change this?
This is still under estimation, 300ms -500ms is a good value to adapt 
the test, this should be configurable.
>> +
>>   /* Global statistics */
>>   static uint32_t rsc_chain_no_mem;
>>   
>>   /* Switcher to enable/disable rsc */
>>   static bool virtio_net_rsc_bypass;
>>   
>> +static uint32_t rsc_timeout = RSC_TIMER_INTERVAL;
>> +
>>   /* Coalesce callback for ipv4/6 */
>>   typedef int32_t (VirtioNetCoalesce) (NetRscChain *chain, NetRscSeg *seg,
>>                                        const uint8_t *buf, size_t size);
>> @@ -1625,6 +1630,35 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f,
>>       return 0;
>>   }
>>   
>> +static void virtio_net_rsc_purge(void *opq)
>> +{
>> +    int ret = 0;
>> +    NetRscChain *chain = (NetRscChain *)opq;
>> +    NetRscSeg *seg, *rn;
>> +
>> +    QTAILQ_FOREACH_SAFE(seg, &chain->buffers, next, rn) {
>> +        if (!qemu_can_send_packet(seg->nc)) {
>> +            /* Should quit or continue? not sure if one or some
>> +            * of the queues fail would happen, try continue here */
> This looks wrong, qemu_can_send_packet() is used for nc's peer not nc
> itself.
OK.
>
>> +            continue;
>> +        }
>> +
>> +        ret = virtio_net_do_receive(seg->nc, seg->buf, seg->size);
>> +        QTAILQ_REMOVE(&chain->buffers, seg, next);
>> +        g_free(seg->buf);
>> +        g_free(seg);
>> +
>> +        if (ret == 0) {
>> +            /* Try next queue */
> Try next seg?
Yes, it's seg.
>
>> +            continue;
>> +        }
> Why need above?
yes, it's optional, my maybe can help if there are extra codes after 
this, will remove this.
>
>> +    }
>> +
>> +    if (!QTAILQ_EMPTY(&chain->buffers)) {
>> +        timer_mod(chain->drain_timer,
>> +                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + rsc_timeout);
> Need stop/start the timer during vm stop/start to save cpu.
Thanks, do you know where should i add the code?
>
>> +    }
>> +}
>>   
>>   static void virtio_net_rsc_cleanup(VirtIONet *n)
>>   {
>> @@ -1810,6 +1844,8 @@ static size_t virtio_net_rsc_callback(NetRscChain *chain, NetClientState *nc,
>>           if (!virtio_net_rsc_cache_buf(chain, nc, buf, size)) {
>>               return 0;
>>           } else {
>> +            timer_mod(chain->drain_timer,
>> +                      qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + rsc_timeout);
>>               return size;
>>           }
>>       }
>> @@ -1877,6 +1913,8 @@ static NetRscChain *virtio_net_rsc_lookup_chain(NetClientState *nc,
>>       }
>>   
>>       chain->proto = proto;
>> +    chain->drain_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
>> +                                      virtio_net_rsc_purge, chain);
>>       chain->do_receive = virtio_net_rsc_receive4;
>>   
>>       QTAILQ_INIT(&chain->buffers);
>