Re: [XENVIF PATCH] Skip hash update if frontend is not enabled

[Owen Smith <owen.smith@xxxxxxxxx>]

It is odd that the controller is blocking when it's connected, but it's possible that it's acquired the lock, while processing but will be immediately disconnected...

Approving this, as the logging would indicate that the patch would resolve this issue, and report success back to NDIS rather than STATUS_NOT_SUPPORTED.

 

Reviewed-by: Owen Smith <owen.smith@xxxxxxxxx>

On Tue, Sep 2, 2025 at 11:49 AM Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> wrote:

On 02/09/2025 11:54, Owen Smith wrote:
> I don't think that any updated hash details will be resent if they are
> changed when Frontend->State != FRONTEND_ENABLED. FrontendEnable will
> call __FrontendUpdateHash before it sets Frontend->State
> to FRONTEND_ENABLED.
>
> Controller->Connected is used to guard writing hash data to the backend,
> but returns STATUS_NOT_SUPPORTED in this case, and this means Frontend-
>  >Hash is not updated and the status is reported back to NDIS.
>
> Owen
>

It doesn't trigger 100% of the time, but I found it easier to repro
under debugger.

At the moment of hang, Controller->Connected is still true but
Frontend->State has entered FRONTEND_CONNECTED (see attached log)

> On Thu, Aug 28, 2025 at 11:01 PM Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> wrote:
>
>     NDIS may set OID_GEN_RECEIVE_SCALE_PARAMETERS during an
>     IRP_MN_REMOVE_DEVICE, after the frontend has been torn down. This will
>     cause the ensuing control request to hang.
>
>     Skip __FrontendUpdateHash if the frontend is not enabled. The skip is
>     done here since we still need to update Frontend->Hash in case the
>     frontend is enabled again later.
>
>     Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
>     ---
>       src/xenvif/frontend.c | 3 +++
>       1 file changed, 3 insertions(+)
>
>     diff --git a/src/xenvif/frontend.c b/src/xenvif/frontend.c
>     index 90ec2b1..c497f24 100644
>     --- a/src/xenvif/frontend.c
>     +++ b/src/xenvif/frontend.c
>     @@ -1889,6 +1889,9 @@ __FrontendUpdateHash(
>           ULONG                   Flags;
>           NTSTATUS                status;
>
>     +    if (Frontend->State != FRONTEND_ENABLED)
>     +        goto done;
>     +
>           Controller = __FrontendGetController(Frontend);
>
>           switch (Hash->Algorithm) {
>     --
>     2.51.0.windows.1
>
>
>
>     Ngoc Tu Dinh | Vates XCP-ng Developer
>
>     XCP-ng & Xen Orchestra - Vates solutions
>
>     web: https://vates.tech <https://vates.tech>
>


--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech