[dpdk-dev] [PATCH v3 05/19] vhost: fix error handling when mem table gets updated
i.maximets at samsung.com
Thu Oct 4 17:11:54 CEST 2018
On 04.10.2018 18:06, Maxime Coquelin wrote:
> On 10/04/2018 04:59 PM, Ilya Maximets wrote:
>> On 04.10.2018 11:13, Maxime Coquelin wrote:
>>> When the memory table gets updated, the rings addresses need
>>> to be translated again. If it fails, we need to exit cleanly
>>> by unmapping memory regions.
>>> Fixes: d5022533c20a ("vhost: retranslate vring addr when memory table changes")
>>> Cc: stable at dpdk.org
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin at redhat.com>
>> Acked-by: Ilya Maximets <i.maximets at samsung.com>
>> Minor comments inline.
>>> lib/librte_vhost/vhost_user.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>>> index 8ffe5aa66..b6eae8dc5 100644
>>> --- a/lib/librte_vhost/vhost_user.c
>>> +++ b/lib/librte_vhost/vhost_user.c
>>> @@ -964,7 +964,8 @@ vhost_user_set_mem_table(struct virtio_net **pdev, struct VhostUserMsg *msg)
>>> dev = translate_ring_addresses(dev, i);
>>> if (!dev)
>>> - return VH_RESULT_ERR;
>>> + goto err_mmap;
>> 1. No need to have two empty lines. (You could fix this while applying)
> Right. I will either fix it while applying, or it will be in next
> revision, if any.
>> 2. In current code, error on message handling will cause disconnect and
>> memory regions will be freed anyway. So, the change is not very
>> important for master (maybe just for consistency with surrounding
>> code) but it could be important for stable versions.
> I may not disconnect directly, as if reply-ack feature is negotiated,
> the slave would reply with a NACK and function handler will return 0.
> I guess that in that case the master (QEMU) will disconnect anyway, but
> this is just an assumption from slave point of view.
Yes, you're right. Thanks.
> : REPLY_ACK protocol feature is only advertised when IOMMU support is
> requested because of a bug in an old upstream QEMU version.
>>> *pdev = dev;
More information about the dev