Re: virtio: why no full reset on virtio_set_status 0 ?

[Alex Bennée]

Claudio Fontana <cfontana@suse.de> writes:

> On 7/29/22 16:00, Claudio Fontana wrote:
>> On 7/29/22 15:21, Alex Bennée wrote:
>>>
>>> Claudio Fontana <cfontana@suse.de> writes:
>>>
>>>> On 7/29/22 12:13, Michael S. Tsirkin wrote:
>>>>> On Fri, Jul 29, 2022 at 11:46:05AM +0200, Claudio Fontana wrote:
>>>>>>>> @@ -2025,7 +2031,6 @@ void virtio_reset(void *opaque)
>>>>>>>>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>>>>>>>>      int i;
>>>>>>>>  
>>>>>>>> -    virtio_set_status(vdev, 0);
>>>>>>>>      if (current_cpu) {
>>>>>>>>          /* Guest initiated reset */
>>>>>>>>          vdev->device_endian = virtio_current_cpu_endian();
>>>>>>>> -- 
>>>>>>>> 2.26.2
>>>>>>>
>>>>>>> As you say this is incomplete ... bout could you share a bit more
>>>>>>> of what issue does this address?
>>>>>>>
>>>>>>
>>>>>> Hi, the problem I am trying to address is a segfault in OVS/dpdk that 
>>>>>> looks like this:
>>>>>
>>>>> Sorry I was not clear. What I mean is, you don't yet know why does 
>>>>> removing
>>>>> virtio_set_status call here prevent the crash in ovs, do you?
>>>>>
>>>>
>>>> I have no idea. Trying to collect logs to figure things out, but as
>>>> mentioned the logs easily hide the issue.
>>>> Likely there is just more to study here.
>>>
>>> Given the OVS is going off on a NULL ptr deref could it just be it's not
>>> handling the disabling/reenabling of the virtqueues as you pause and
>>> restart properly? I could certainly imagine a backend jumping the gun to
>>> read a queue going very wrong if the current queue state is disabled.
>>>
>> 
>> In this case both the ovs buf_addr and buf_iova are NULL, which is a
>> nice case as they are more detectable,
>> however I also have segfaults where the addresses are just garbage.
>> 
>> I wonder whether it's possible that given the fact that the guest is
>> going away without notification (SIGKILL),
>> as the guest driver resets the device and communicates with QEMU,
>> QEMU adapts the state without notifying ovs,
>> so ovs happily tries to dequeue data from memory that isn't there. But I am 
>> just guessing.
>> 
>> I am still studying the qemu vhost user side and ovs/dpdk side to
>> try to understand how this whole thing works.
>> 
>> Thanks,
>> 
>> CLaudio
>> 
>
> I am pursuing this as a DPDK library issue.
>
> It would be cool to have ovs, dpdk and vhost-user with the default
> test-pmd application somehow hooked up in a basic test
> in one of these projects..

I agree although it's hard to marshal multiple projects into a known
working state that isn't too brittle for CI purposes. The existing
qos-test testing doesn't really exercise any more than the initial setup
and register reading of the VirtIO device.

For example we have a number of non network standalone vhost-user
backends in rust-vmm which would be nice to plumb in somehow.

>
> Thanks,
>
> Claudio


-- 
Alex Bennée