Re: [PATCH] Make sure Frontend->NumQueues is not zero

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 12/07/2021 17:07, Owen Smith wrote:
> - Checks multi-queue-max-queues is non-zero
> - ASSERTs Frontend->NumQueues is non-zero in the accessor
>
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>   src/xenvif/frontend.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/xenvif/frontend.c b/src/xenvif/frontend.c
> index 30de494..f5cb382 100644
> --- a/src/xenvif/frontend.c
> +++ b/src/xenvif/frontend.c
> @@ -1785,6 +1785,9 @@ FrontendSetNumQueues(
>           XENBUS_STORE(Free,
>                        &Frontend->StoreInterface,
>                        Buffer);
> +
> +        if (BackendMaxQueues == 0)
> +            BackendMaxQueues = 1;
This means that the backend has deliberately set the node in xenstore to 
zero. Why do we need to tolerate that? I would have thought refusing to 
connect would be the better option in the face of such a blatant bug.
>       } else {
>           BackendMaxQueues = 1;
>       }
> @@ -1800,6 +1803,7 @@ __FrontendGetNumQueues(
>       IN  PXENVIF_FRONTEND    Frontend
>       )
>   {
> +    ASSERT(Frontend->NumQueues != 0);
>       return Frontend->NumQueues;
>   }
>   
> @@ -1953,7 +1957,6 @@ FrontendSetHashAlgorithm(
>       case XENVIF_PACKET_HASH_ALGORITHM_TOEPLITZ:
>           // Don't allow toeplitz hashing to be configured for a single
>           // queue, or if it has been explicitly disabled
> -        ASSERT(__FrontendGetNumQueues(Frontend) != 0);
>           status = (__FrontendGetNumQueues(Frontend) == 1 ||
>                     Frontend->DisableToeplitz != 0) ?
>                    STATUS_NOT_SUPPORTED :
> @@ -2358,7 +2361,7 @@ fail4:
>       MacDisconnect(__FrontendGetMac(Frontend));
>   
>       Frontend->Split = FALSE;
> -    Frontend->NumQueues = 0;
> +    Frontend->NumQueues = 1;
>   
>   fail3:
>       Error("fail3\n");
> @@ -2395,7 +2398,7 @@ FrontendDisconnect(
>       MacDisconnect(__FrontendGetMac(Frontend));
>   
>       Frontend->Split = FALSE;
> -    Frontend->NumQueues = 0;
> +    Frontend->NumQueues = 1;
This seems wrong. We're disconnecting, which means there are - by 
definition - no queues.
  Paul

>   
>       XENBUS_DEBUG(Deregister,
>                    &Frontend->DebugInterface,
> @@ -3063,6 +3066,7 @@ FrontendTeardown(
>       RtlZeroMemory(&Frontend->Lock, sizeof (KSPIN_LOCK));
>   
>       Frontend->BackendDomain = 0;
> +    Frontend->NumQueues = 0;
>   
>       __FrontendFree(Frontend->Prefix);
>       Frontend->Prefix = NULL;