Re: [XENVIF PATCH] Invalidate FDOs when no devices are present

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 02/12/2024 09:37, Tu Dinh wrote:
> In cases where the entire xenstore device key is removed, FdoScan()
> currently skips reenumeration entirely. This causes it to not pass the
> removal event to xennet. Fix the issue by reenumerating devices even if
> xenstore key does not exist.
>
> Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
> ---
>   src/xenvif/fdo.c | 98 ++++++++++++++++++++++++++----------------------
>   1 file changed, 54 insertions(+), 44 deletions(-)
Acked-by: Paul Durrant <paul@xxxxxxx>

> diff --git a/src/xenvif/fdo.c b/src/xenvif/fdo.c
> index d181184..2499563 100644
> --- a/src/xenvif/fdo.c
> +++ b/src/xenvif/fdo.c
> @@ -758,18 +758,20 @@ __FdoEnumerate(
>               Name = PdoGetName(Pdo);
>               Missing = TRUE;
>   
> -            // If the PDO already exists and its name is in the device list
> -            // then we don't want to remove it.
> -            for (Index = 0; Devices[Index].Buffer != NULL; Index++) {
> -                PANSI_STRING Device = &Devices[Index];
> -
> -                if (Device->Length == 0)
> -                    continue;
> -
> -                if (strcmp(Name, Device->Buffer) == 0) {
> -                    Missing = FALSE;
> -                    Device->Length = 0;  // avoid duplication
> -                    break;
> +            if (Devices != NULL) {
> +                // If the PDO already exists and its name is in the device list
> +                // then we don't want to remove it.
> +                for (Index = 0; Devices[Index].Buffer != NULL; Index++) {
> +                    PANSI_STRING Device = &Devices[Index];
> +
> +                    if (Device->Length == 0)
> +                        continue;
> +
> +                    if (strcmp(Name, Device->Buffer) == 0) {
> +                        Missing = FALSE;
> +                        Device->Length = 0;  // avoid duplication
> +                        break;
> +                    }
>                   }
>               }
>   
> @@ -797,40 +799,42 @@ __FdoEnumerate(
>       }
>   
>       // Walk the class list and create PDOs for any new device
> -    for (Index = 0; Devices[Index].Buffer != NULL; Index++) {
> -        PANSI_STRING Device = &Devices[Index];
> +    if (Devices != NULL) {
> +        for (Index = 0; Devices[Index].Buffer != NULL; Index++) {
> +            PANSI_STRING Device = &Devices[Index];
>   
> -        if (Device->Length != 0) {
> -            ULONG   Number;
> -            CHAR    Prefix[sizeof ("device/vif/XXXXXXXXXX")];
> -            PCHAR   Address;
> +            if (Device->Length != 0) {
> +                ULONG   Number;
> +                CHAR    Prefix[sizeof ("device/vif/XXXXXXXXXX")];
> +                PCHAR   Address;
>   
> -            Number = strtol(Device->Buffer, NULL, 10);
> +                Number = strtol(Device->Buffer, NULL, 10);
>   
> -            status = RtlStringCbPrintfA(Prefix,
> -                                        sizeof (Prefix),
> -                                        "device/vif/%u",
> -                                        Number);
> -            ASSERT(NT_SUCCESS(status));
> -            if (!NT_SUCCESS(status))
> -                continue;
> -
> -            status = XENBUS_STORE(Read,
> -                                  &Fdo->StoreInterface,
> -                                  NULL,
> -                                  Prefix,
> -                                  "mac",
> -                                  &Address);
> -            if (!NT_SUCCESS(status))
> -                continue;
> +                status = RtlStringCbPrintfA(Prefix,
> +                                            sizeof (Prefix),
> +                                            "device/vif/%u",
> +                                            Number);
> +                ASSERT(NT_SUCCESS(status));
> +                if (!NT_SUCCESS(status))
> +                    continue;
>   
> -            status = PdoCreate(Fdo, Number, Address);
> -            if (NT_SUCCESS(status))
> -                NeedInvalidate = TRUE;
> +                status = XENBUS_STORE(Read,
> +                                      &Fdo->StoreInterface,
> +                                      NULL,
> +                                      Prefix,
> +                                      "mac",
> +                                      &Address);
> +                if (!NT_SUCCESS(status))
> +                    continue;
>   
> -            XENBUS_STORE(Free,
> -                         &Fdo->StoreInterface,
> -                         Address);
> +                status = PdoCreate(Fdo, Number, Address);
> +                if (NT_SUCCESS(status))
> +                    NeedInvalidate = TRUE;
> +
> +                XENBUS_STORE(Free,
> +                            &Fdo->StoreInterface,
> +                            Address);
> +            }
>           }
>       }
>   
> @@ -978,8 +982,12 @@ FdoScan(
>               Devices = NULL;
>           }
>   
> -        if (Devices == NULL)
> -            goto loop;
> +        if (Devices == NULL) {
> +            if (status == STATUS_OBJECT_PATH_NOT_FOUND)
> +                goto invalidate;
> +            else
> +                goto loop;
> +        }
>   
>           if (ParametersKey != NULL) {
>               status = RegistryQuerySzValue(ParametersKey,
> @@ -1019,9 +1027,11 @@ FdoScan(
>           if (UnsupportedDevices != NULL)
>               RegistryFreeSzValue(UnsupportedDevices);
>   
> +invalidate:
>           NeedInvalidate = __FdoEnumerate(Fdo, Devices);
>   
> -        __FdoFreeAnsi(Devices);
> +        if (Devices != NULL)
> +            __FdoFreeAnsi(Devices);
>   
>           if (NeedInvalidate) {
>               NeedInvalidate = FALSE;