On 11/19/21 07:30, Li, Miao wrote:
Hi
-----Original Message-----
From: Maxime Coquelin <maxime.coque...@redhat.com>
Sent: Tuesday, November 16, 2021 5:36 PM
To: Li, Miao <miao...@intel.com>; dev@dpdk.org
Cc: Xia, Chenbo <chenbo....@intel.com>
Subject: Re: [PATCH v1] net/vhost: add queue status check
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.
If we don't add this check, rte_vhost_get_monitor_addr will return -EINVAL when check if
dev is null. But before return, get_device() will be called and print error log
"device not found". So we want to add this check and return -EINVAL before call
rte_vhost_get_monitor_addr. If we don't add this check, the vhost monitor address will
also not be got but vhost will print error log continuously. It have no function impact,
so I think it is not a fix.
Signed-off-by: Miao Li <miao...@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.
I don't know which return value will be better. Do you have any suggestions?
Thanks!
How does it help?
What does prevent allow_queuing to become zero between the check and the
call to rte_vhost_get_monitor_addr?
This check will prevent vhost to print error log continuously.
You mean, it will prevent it most of the time, as there is still a
window where it can happen, if allow_queueing is set between is 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.
I think the while_queuing is unnecessary because we only read the value in vq
and this API will only be called as a callback of RX.
Thanks,
Miao
ret = rte_vhost_get_monitor_addr(vq->vid, vq->virtqueue_id,
&vhost_pmc);
if (ret < 0)
Maxime
