Re: [PATCH v1] net/vhost: add queue status check

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

On 11/16/21 10:34, Maxime Coquelin wrote:
> 
> 
> On 11/16/21 17:44, Miao Li wrote:
>> This patch adds queue status check to make sure that vhost monitor
>> address will not be got until the link between backend and frontend
> s/got/gone/?
>> up and the packets are allowed to be queued.
> 
> It needs a fixes tag.
> 
>> Signed-off-by: Miao Li <miao.li@intel.com>
>> ---
>>   drivers/net/vhost/rte_eth_vhost.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/vhost/rte_eth_vhost.c 
>> b/drivers/net/vhost/rte_eth_vhost.c
>> index 070f0e6dfd..9d600054d8 100644
>> --- a/drivers/net/vhost/rte_eth_vhost.c
>> +++ b/drivers/net/vhost/rte_eth_vhost.c
>> @@ -1415,6 +1415,8 @@ vhost_get_monitor_addr(void *rx_queue, struct 
>> rte_power_monitor_cond *pmc)
>>       int ret;
>>       if (vq == NULL)
>>           return -EINVAL;
>> +    if (unlikely(rte_atomic32_read(&vq->allow_queuing) == 0))
>> +        return -EINVAL;

Also, EINVAL might not be the right return value here.

> How does it help?
> What does prevent allow_queuing to become zero between the check and the
> call to rte_vhost_get_monitor_addr?
> 
> I think you need to implement the same logic as in eth_vhost_rx(), i.e.
> check allow_queueing, set while_queueing, check allow_queueing, do your
> stuff and clear while_queuing.
> 
>>       ret = rte_vhost_get_monitor_addr(vq->vid, vq->virtqueue_id,
>>               &vhost_pmc);
>>       if (ret < 0)
>>
> 
> Maxime