Re: [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain

[Jason Wang <jasowang@redhat.com>]

On 2018年10月15日 16:03, Wei Xu wrote:
> On Mon, Oct 15, 2018 at 03:45:46PM +0800, Jason Wang wrote:
>>
>> On 2018年10月11日 22:08, wexu@redhat.com wrote:
>>> From: Wei Xu <wexu@redhat.com>
>>>
>>> With the support of marking a descriptor used/unused in 'flags'
>>> field for 1.1, the current way of filling a chained descriptors
>>> does not work since driver side may get the wrong 'num_buffer'
>>> information in case of the head descriptor has been filled in
>>> while the subsequent ones are still in processing in device side.
>>>
>>> This patch fills the head one after done all the others one.
>>>
>>> Signed-off-by: Wei Xu <wexu@redhat.com>
>>> ---
>>>   hw/net/virtio-net.c | 11 ++++++++++-
>>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index 4bdd5b8..186c86cd2 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -1198,6 +1198,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>>>       struct virtio_net_hdr_mrg_rxbuf mhdr;
>>>       unsigned mhdr_cnt = 0;
>>>       size_t offset, i, guest_offset;
>>> +    VirtQueueElement head;
>>> +    int head_len = 0;
>>>       if (!virtio_net_can_receive(nc)) {
>>>           return -1;
>>> @@ -1275,7 +1277,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>>>           }
>>>           /* signal other side */
>>> -        virtqueue_fill(q->rx_vq, elem, total, i++);
>>> +        if (i == 0) {
>>> +            head_len = total;
>>> +            head = *elem;
>>> +        } else {
>>> +            virtqueue_fill(q->rx_vq, elem, len, i);
>>> +        }
>>> +        i++;
>>>           g_free(elem);
>>>       }
>>> @@ -1286,6 +1294,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>>>                        &mhdr.num_buffers, sizeof mhdr.num_buffers);
>>>       }
>>> +    virtqueue_fill(q->rx_vq, &head, head_len, 0);
>> It's not a good idea to fix API in device implementation. Let's introduce
>> new API and fix it there.
>>
>> E.g virtqueue_fill_n() and update the flag of first elem at the last step.
> OK, I haven't considered about the other devices so far.

It will be an issue if they use virtqueue_fill() directly.

Thanks

>
> Wei
>
>> Thanks
>>
>>>       virtqueue_flush(q->rx_vq, i);
>>>       virtio_notify(vdev, q->rx_vq);
>>