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 <yan...@cn.fujitsu.com>
>>> ---
>>> 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;
>>
>>
>> .
>>
>
