[dpdk-dev] [PATCH 4/6] vhost: workaround stale vring base
Michael S. Tsirkin
mst at redhat.com
Mon May 9 14:19:13 CEST 2016
On Fri, May 06, 2016 at 11:40:22PM -0700, Yuanhan Liu wrote:
> When DPDK app crashes (or quits, or gets killed), and when QEMU supports
> reconnecting (patches have been sent, not merged yet), a restart of DPDK
> app would get stale vring base from QEMU. That would break the kernel
> virtio net completely, making it non-work any more, unless a driver reset
> is done.
>
> So, instead of getting the stale vring base from QEMU, Huawei suggested
> we could get a proper one from used->idx.
>
> Cc: "Michael S. Tsirkin" <mst at redhat.com>
> Suggested-by: "Xie, Huawei" <huawei.xie at intel.com>
> Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> ---
>
> Note that a right way to handle reconnect from abnormal quit would be
> let the guest driver to initiate a reset when reconnect is detected
> from QEMU. As a reset from the virtio net driver would resets all virtio
> related meta datas, and trigger the vhost-user re-initiation again,
> therefore, it would make the reconnect work as expected.
>
> What's unforunate is that driver reset on reconnect, as the term "driver"
> implies, need driver hackings, which could be a linux kernel virtio net
> driver, DPDK pmd driver, or even, the windows driver. Apparently, it will
> not work so far, or even for a long while, until we are adapted to the
> new kernel.
>
> The fix (or more precisely, the workaround) from this patch would make
> reconnect work in most case, including the following two hypothesis that
> might corrupt virtio memory:
>
> - vring_used_elem might be in stale state when we are in the middle of
> updating used vring. Say, we might have updated the "id" field, but
> leaving "len" untouched.
>
> - vring desc buff might also be in stale state, when we are copying
> data into it.
>
> The reason it still works is that we haven't updated used->idx yet,
> which means guest driver will not start processing those buggy used
> entries. Therefore, no issues.
>
> However, Michael claims some concerns: he made a good point: a crash
> is happening means some memory is corrupted, and it could be the virtio
> memory being corrupted. In such case, nothing will work without the
> reset.
>
> But I would say, that an app in product use would rare crash, and even
> if it crashes, the chance that virtio memory being corrupted would be
> relatively smaller then. Besides that, it would work, say when the app
> is killed by ctrl-c or kill command. So, it works in the most cases.
> But still, I would say it's just a workaround so far.
>
> On the other hand, without this patch, it would be 100% not working
> from abnormal quit. But with this patch, it works in most cases, just
> don't work in a rare case that when virtio memory is corrupted. Therefore,
> I'd say, it is still good to have, until we have a perfect solution.
> ---
> lib/librte_vhost/virtio-net.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index c88aaa3..df103aa 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -560,6 +560,14 @@ vhost_set_vring_addr(int vid, struct vhost_vring_addr *addr)
> return -1;
> }
>
> + if (vq->last_used_idx != vq->used->idx) {
> + RTE_LOG(WARNING, VHOST_CONFIG,
> + "last_used_idx (%u) and vq->used->idx (%u) mismatch\n",
> + vq->last_used_idx, vq->used->idx);
> + vq->last_used_idx = vq->used->idx;
> + vq->last_used_idx_res = vq->used->idx;
> + }
> +
> vq->log_guest_addr = addr->log_guest_addr;
>
> LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n",
Acked-by: Michael S. Tsirkin <mst at redhat.com>
And I would go further and just ignore the index coming from QEMU.
dpdk processes packets in order so it does not need it.
> --
> 1.9.0
More information about the dev
mailing list