Re: [dpdk-dev] [PATCH v7 5/8] net/virtio: implement transmit path for packed queues

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 10/12/2018 09:24 AM, Jens Freimann wrote:
> On Thu, Oct 11, 2018 at 07:31:57PM +0200, Maxime Coquelin wrote:
>>
>> I'm testing your series, and it gets stuck after 256 packets in transmit
>> path. When it happens, descs flags indicate it has been made available
>> by the driver (desc->flags = 0x80), but it is not consistent with the
>> expected wrap counter value (0).
>>
>> Not sure this is the root cause, but it seems below code is broken:
> 
>>
>> On 10/03/2018 03:11 PM, Jens Freimann wrote:
> 
> [snip]
>> +
>>> +    do {
>>> +        if (idx >= vq->vq_nentries) {
>>> +            idx = 0;
>>> +            vq->vq_ring.avail_wrap_counter ^= 1;
>>> +        }
>>> +        start_dp[idx].addr  = VIRTIO_MBUF_DATA_DMA_ADDR(cookie, vq);
>>> +        start_dp[idx].len   = cookie->data_len;
>>> +        start_dp[idx].flags = cookie->next ? VRING_DESC_F_NEXT : 0;
>>> +        start_dp[idx].flags |=
>>> +            VRING_DESC_F_AVAIL(vq->vq_ring.avail_wrap_counter) |
>>> +            VRING_DESC_F_USED(!vq->vq_ring.avail_wrap_counter);
>>> +        if (use_indirect) {
>>> +            if (++idx >= (seg_num + 1))
>>> +                break;
>>> +        } else {
>>> +            dxp = &vq->vq_descx[idx];
>>> +            idx = dxp->next;
>>> +        }
>>
>> Imagine current idx is 255, dxp->next will give idx 0, right?
> 
> No it will be VQ_RING_DESC_CHAIN_END which is defined as 32768.
>> In that case, for desc[0], on next iteration, the flags won't be set 
>> available properly, as vq->vq_ring.avail_wrap_counter isn't updated.
> 
> It will wrap because VQ_RING_DESC_CHAIN_END is > ring size.

I'm not sure to understand. I dig a bit deeper into the code and may
come back if I have questions.

> I can't reproduce what you see. Do you test with txonly fwd mode?

Yes, txonly fwd mode.

On host side, I have latest master with my 2 patches series enabling
packed ring in vhost backend and Tiwei's patch fixing notification
disablement.

For QEMU, I use the branch you shared and started it with below cmdline:
./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 3072 -smp 3 
-machine q35 -cpu host   -chardev socket,id=char0,path=/tmp/vhost-user1 
  -netdev type=vhost-user,id=hn2,chardev=char0,vhostforce,queues=1 
-device 
virtio-net-pci,netdev=hn2,id=v0,mq=off,mrg_rxbuf=off,ring_packed=on,mac=52:54:00:11:22:11 
   -object 
memory-backend-file,id=mem,size=3G,mem-path=/dev/hugepages,share=on 
-numa node,memdev=mem -mem-prealloc   -k fr   -net 
user,hostfwd=tcp::10021-:22 -net nic   -serial stdio 
/home/virt/rhel7.6-1.qcow2

Regards,
Maxime

> regards,
> Jens