qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v11 04/12] net: merge qemu_deliver_packet and qe


From: Jason Wang
Subject: Re: [Qemu-devel] [PATCH v11 04/12] net: merge qemu_deliver_packet and qemu_deliver_packet_iov
Date: Tue, 22 Sep 2015 16:14:39 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0


On 09/22/2015 03:44 PM, Yang Hongyang wrote:
> Hi Jason,
>
>   Thanks for the review,
>
> On 09/22/2015 03:30 PM, Jason Wang wrote:
>>
>>
>> On 09/16/2015 08:16 PM, Yang Hongyang wrote:
>>> qemu_deliver_packet_iov already have the compat delivery, we
>>> can drop qemu_deliver_packet.
>>>
>>> Signed-off-by: Yang Hongyang <address@hidden>
>>> ---
>>>   include/net/net.h |  5 -----
>>>   net/net.c         | 40 +++++++---------------------------------
>>>   net/queue.c       |  6 +++++-
>>>   3 files changed, 12 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/include/net/net.h b/include/net/net.h
>>> index 36e5fab..7af3e15 100644
>>> --- a/include/net/net.h
>>> +++ b/include/net/net.h
>>> @@ -152,11 +152,6 @@ void qemu_check_nic_model(NICInfo *nd, const
>>> char *model);
>>>   int qemu_find_nic_model(NICInfo *nd, const char * const *models,
>>>                           const char *default_model);
>>>
>>> -ssize_t qemu_deliver_packet(NetClientState *sender,
>>> -                            unsigned flags,
>>> -                            const uint8_t *data,
>>> -                            size_t size,
>>> -                            void *opaque);
>>>   ssize_t qemu_deliver_packet_iov(NetClientState *sender,
>>>                               unsigned flags,
>>>                               const struct iovec *iov,
>>> diff --git a/net/net.c b/net/net.c
>>> index ad37419..ca35b27 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -597,36 +597,6 @@ static ssize_t filter_receive(NetClientState
>>> *nc, NetFilterChain chain,
>>>       return filter_receive_iov(nc, chain, sender, flags, &iov, 1,
>>> sent_cb);
>>>   }
>>>
>>> -ssize_t qemu_deliver_packet(NetClientState *sender,
>>> -                            unsigned flags,
>>> -                            const uint8_t *data,
>>> -                            size_t size,
>>> -                            void *opaque)
>>> -{
>>> -    NetClientState *nc = opaque;
>>> -    ssize_t ret;
>>> -
>>> -    if (nc->link_down) {
>>> -        return size;
>>> -    }
>>> -
>>> -    if (nc->receive_disabled) {
>>> -        return 0;
>>> -    }
>>> -
>>> -    if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) {
>>> -        ret = nc->info->receive_raw(nc, data, size);
>>> -    } else {
>>> -        ret = nc->info->receive(nc, data, size);
>>> -    }
>>> -
>>> -    if (ret == 0) {
>>> -        nc->receive_disabled = 1;
>>> -    }
>>> -
>>> -    return ret;
>>> -}
>>> -
>>>   void qemu_purge_queued_packets(NetClientState *nc)
>>>   {
>>>       if (!nc->peer) {
>>> @@ -717,14 +687,18 @@ ssize_t qemu_send_packet_raw(NetClientState
>>> *nc, const uint8_t *buf, int size)
>>>   }
>>>
>>>   static ssize_t nc_sendv_compat(NetClientState *nc, const struct
>>> iovec *iov,
>>> -                               int iovcnt)
>>> +                               int iovcnt, unsigned flags)
>>>   {
>>>       uint8_t buffer[NET_BUFSIZE];
>>>       size_t offset;
>>>
>>>       offset = iov_to_buf(iov, iovcnt, 0, buffer, sizeof(buffer));
>>>
>>> -    return nc->info->receive(nc, buffer, offset);
>>> +    if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) {
>>> +        return nc->info->receive_raw(nc, buffer, offset);
>>> +    } else {
>>> +        return nc->info->receive(nc, buffer, offset);
>>> +    }
>>
>> Then for net clients that doesn't support receive_iov, an extra memcpy
>> is introduced. This seems unnecessary. And this patch looks independent,
>> so please drop this patch from the series (This can also help to reduce
>> the iteration). I think we may need this after all net clients support
>> receive_iov().
>
> This patch is required by the next patch which introduce a
> +/* Returns:
> + *   >0 - success
> + *    0 - queue packet for future redelivery
> + *   <0 - failure (discard packet)
> + */
> +typedef ssize_t (NetQueueDeliverFunc)(NetClientState *sender,
> +                                      unsigned flags,
> +                                      const struct iovec *iov,
> +                                      int iovcnt,
> +                                      void *opaque);
>
> If we drop this patch, we will need to introduce 2 deliver func, which
> seems not clean and simple.

Then you can probably skip the iov_to_buf() if iov_cnt is one in the
above code?

>
>>
>>
>>>   }
>>>
>>>   ssize_t qemu_deliver_packet_iov(NetClientState *sender,
>>> @@ -747,7 +721,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState
>>> *sender,
>>>       if (nc->info->receive_iov) {
>>>           ret = nc->info->receive_iov(nc, iov, iovcnt);
>>>       } else {
>>> -        ret = nc_sendv_compat(nc, iov, iovcnt);
>>> +        ret = nc_sendv_compat(nc, iov, iovcnt, flags);
>>>       }
>>>
>>>       if (ret == 0) {
>>> diff --git a/net/queue.c b/net/queue.c
>>> index ebbe2bb..cf8db3a 100644
>>> --- a/net/queue.c
>>> +++ b/net/queue.c
>>> @@ -152,9 +152,13 @@ static ssize_t qemu_net_queue_deliver(NetQueue
>>> *queue,
>>>                                         size_t size)
>>>   {
>>>       ssize_t ret = -1;
>>> +    struct iovec iov = {
>>> +        .iov_base = (void *)data,
>>> +        .iov_len = size
>>> +    };
>>>
>>>       queue->delivering = 1;
>>> -    ret = qemu_deliver_packet(sender, flags, data, size,
>>> queue->opaque);
>>> +    ret = qemu_deliver_packet_iov(sender, flags, &iov, 1,
>>> queue->opaque);
>>>       queue->delivering = 0;
>>>
>>>       return ret;
>>
>>
>> .
>>
>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]