[dpdk-dev] [PATCH] net/virtio: wait device ready in device reset

Xia, Chenbo chenbo.xia at intel.com
Thu Aug 26 09:15:53 CEST 2021


Hi Adrew & Xueming,

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
> Sent: Tuesday, August 24, 2021 11:41 PM
> To: Xueming(Steven) Li <xuemingl at nvidia.com>
> Cc: dev at dpdk.org; Maxime Coquelin <maxime.coquelin at redhat.com>; Xia, Chenbo
> <chenbo.xia at intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/virtio: wait device ready in device reset
> 
> On 8/23/21 4:54 PM, Xueming(Steven) Li wrote:
> >
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
> >> Sent: Monday, August 23, 2021 5:57 PM
> >> To: Xueming(Steven) Li <xuemingl at nvidia.com>
> >> Cc: dev at dpdk.org; Maxime Coquelin <maxime.coquelin at redhat.com>; Chenbo Xia
> <chenbo.xia at intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH] net/virtio: wait device ready in device
> reset
> >>
> >> On 8/23/21 9:39 AM, Xueming Li wrote:
> >>> According to virtio spec, the device MUST reset when 0 is written to
> >>> device_status, and present a 0 in device_status once that is done.
> >>>
> >>> This patch adds the missing part of waiting status 0 in reset function.
> >>>
> >>> Signed-off-by: Xueming Li <xuemingl at nvidia.com>
> >>> ---
> >>>  drivers/net/virtio/virtio.c | 7 +++++--
> >>>  1 file changed, 5 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/virtio/virtio.c b/drivers/net/virtio/virtio.c
> >>> index 7e1e77797f..f003f612d6 100644
> >>> --- a/drivers/net/virtio/virtio.c
> >>> +++ b/drivers/net/virtio/virtio.c
> >>> @@ -3,6 +3,8 @@
> >>>   * Copyright(c) 2020 Red Hat, Inc.
> >>>   */
> >>>
> >>> +#include <unistd.h>
> >>> +
> >>>  #include "virtio.h"
> >>>
> >>>  uint64_t
> >>> @@ -39,8 +41,9 @@ void
> >>>  virtio_reset(struct virtio_hw *hw)
> >>>  {
> >>>  	VIRTIO_OPS(hw)->set_status(hw, VIRTIO_CONFIG_STATUS_RESET);
> >>> -	/* flush status write */
> >>> -	VIRTIO_OPS(hw)->get_status(hw);
> >>> +	/* Flush status write and wait device ready. */
> >>> +	while (VIRTIO_OPS(hw)->get_status(hw) != VIRTIO_CONFIG_STATUS_RESET)
> >>> +		usleep(1000L);
> >>
> >> Don't we need a protection against forever loop here?
> >
> > Good question, ideally we need, kernel driver function vp_reset() seems to
> have same issue.
> 
> Yes, I've seen it.
> 
> > How about leaving an error message before return?
> 
> @Maxime, @Chenbo, what do you think?

I would vote for waiting for some time before return rather than forever loop
and error message is needed.

My understanding is for kernel, it's fine to sleep forever as kernel could schedule
it but for DPDK, it will lead to main lcore unable to do other things but sleep
forever. Meanwhile, users will see the app stuck but don't know what's wrong here.

Thanks,
Chenbo

 


More information about the dev mailing list