Re: [PATCH 2/6] Since Rx/Tx ring disconnects now no longer wait forever, we add some logging to catch those cases where the disconnect has timed out, indicating how many requests are still in the ring. This aids greatly with customer debugging.

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 20/07/2021 14:29, Martin Harvey wrote:
> Signed-Off-By: Martin Harvey <martin.harvey@xxxxxxxxxx>
> ---
>   src/xenvif/receiver.c    | 16 ++++++++++++++++
>   src/xenvif/transmitter.c |  6 ++++++
>   2 files changed, 22 insertions(+)
>
> diff --git a/src/xenvif/receiver.c b/src/xenvif/receiver.c
> index c70d938..7295646 100644
> --- a/src/xenvif/receiver.c
> +++ b/src/xenvif/receiver.c
> @@ -2887,6 +2887,22 @@ __ReceiverRingDisconnect(
>       ASSERT3U(Ring->ResponsesProcessed, ==, Ring->RequestsPushed);
>       ASSERT3U(Ring->RequestsPushed, ==, Ring->RequestsPosted);
>   
> +    //Debug logs as well as assertion, else customer debugging difficult.
> +    if ((Ring->ResponsesProcessed != Ring->RequestsPushed) ||
> +        (Ring->RequestsPushed != Ring->RequestsPosted))
> +    {
> +        Info("%s[%u]: Ring disconnect, counts do not match.\n",
> +            FrontendGetPath(Frontend),
> +            Ring->Index);
> +
> +        Info("%s[%u]: Processed: %u Pushed: %u Posted: %u\n",
> +            FrontendGetPath(Frontend),
> +            Ring->Index,
> +            Ring->ResponsesProcessed,
> +            Ring->RequestsPushed,
> +            Ring->RequestsPosted);
> +    }
> +
>       Ring->ResponsesProcessed = 0;
>       Ring->RequestsPushed = 0;
>       Ring->RequestsPosted = 0;
> diff --git a/src/xenvif/transmitter.c b/src/xenvif/transmitter.c
> index e114061..724615d 100644
> --- a/src/xenvif/transmitter.c
> +++ b/src/xenvif/transmitter.c
> @@ -4010,6 +4010,12 @@ __TransmitterRingDisable(
>       }
>       if (Abort)
>       {
> +        Info("%s[%u]: Timed out disable: Requests pushed: %u, Responses processed: 
> %u.\n",
> +            FrontendGetPath(Frontend),
> +            Ring->Index,
> +            Ring->RequestsPushed,
> +            Ring->ResponsesProcessed);
> +
Did you consider triggering a debug dump (at least of the transmitter) 
rather than adding more ad-hoc logging?
  Paul

>           __TransmitterRingFakeResponses(Ring);
>           (VOID) TransmitterRingPoll(Ring);
>       }