DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Jens Freimann <jfreimann@redhat.com>
Cc: dev@dpdk.org, tiwei.bie@intel.com, Gavin.Hu@arm.com
Subject: Re: [dpdk-dev] [PATCH v7 5/8] net/virtio: implement transmit path for packed queues
Date: Fri, 12 Oct 2018 09:41:41 +0200	[thread overview]
Message-ID: <34415e7c-ad8d-60f4-2b5f-c551a3270557@redhat.com> (raw)
In-Reply-To: <20181012072429.frizbuqdzn5u6byv@jenstp.localdomain>



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

  reply	other threads:[~2018-10-12  7:41 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-03 13:11 [dpdk-dev] [PATCH v7 0/8] implement packed virtqueues Jens Freimann
2018-10-03 13:11 ` [dpdk-dev] [PATCH v7 1/8] net/virtio: vring init for packed queues Jens Freimann
2018-10-04 11:54   ` Maxime Coquelin
2018-10-05  8:10     ` Jens Freimann
2018-10-03 13:11 ` [dpdk-dev] [PATCH v7 2/8] net/virtio: add packed virtqueue defines Jens Freimann
2018-10-04 11:54   ` Maxime Coquelin
2018-10-03 13:11 ` [dpdk-dev] [PATCH v7 3/8] net/virtio: add packed virtqueue helpers Jens Freimann
2018-10-03 13:11 ` [dpdk-dev] [PATCH v7 4/8] net/virtio: dump packed virtqueue data Jens Freimann
2018-10-04 13:23   ` Maxime Coquelin
2018-10-03 13:11 ` [dpdk-dev] [PATCH v7 5/8] net/virtio: implement transmit path for packed queues Jens Freimann
2018-10-10  7:27   ` Maxime Coquelin
2018-10-10 11:43     ` Jens Freimann
2018-10-11 17:31   ` Maxime Coquelin
2018-10-12  7:24     ` Jens Freimann
2018-10-12  7:41       ` Maxime Coquelin [this message]
2018-10-03 13:11 ` [dpdk-dev] [PATCH v7 6/8] net/virtio: implement receive " Jens Freimann
2018-10-03 13:11 ` [dpdk-dev] [PATCH v7 7/8] net/virtio: add virtio send command packed queue support Jens Freimann
2018-10-03 13:11 ` [dpdk-dev] [PATCH v7 8/8] net/virtio: enable packed virtqueues by default Jens Freimann
2018-10-03 13:19 ` [dpdk-dev] [PATCH v7 0/8] implement packed virtqueues Jens Freimann
2018-10-04 13:59 ` Maxime Coquelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=34415e7c-ad8d-60f4-2b5f-c551a3270557@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=Gavin.Hu@arm.com \
    --cc=dev@dpdk.org \
    --cc=jfreimann@redhat.com \
    --cc=tiwei.bie@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).