Re: [XENVIF PATCH] Fix argument to ControllerSetHashAlgorithm

["Tu Dinh" <ngoc-tu.dinh@xxxxxxxxxx>]

On 23/07/2025 10:32, Owen Smith wrote:
> XENVIF_PACKET_HASH_ALGORITHM_UNSPECIFIED seems to be a valid value, used
> to determine if xennet requested or did not request a specific
> algorithm, so I think making XENVIF_PACKET_HASH_ALGORITHM_UNSPECIFIED
> report STATUS_NOT_SUPPORTED could cause issues.
>
> The mapping from XENVIF_PACKET_HASH_ALGORITHM values
> to XEN_NETIF_CTRL_HASH_ALGORITHM_* is a good fix.

I don't mind reverting the behavior, but I couldn't find any mention of
XENVIF_PACKET_HASH_ALGORITHM_UNSPECIFIED in xennet, Windows also only
supports NdisHashFunctionToeplitz. Was this used in a previous xennet
version? Also, the default case may very well break if other hash
algorithms were introduced in the future...

>
> Owen
>
> On Wed, Jul 23, 2025 at 8:53 AM Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> wrote:
>
>     ControllerSetHashAlgorithm expects a XEN_NETIF_CTRL_HASH_ALGORITHM_*
>     whereas Hash->Algorithm is of type XENVIF_PACKET_HASH_ALGORITHM.
>
>     These two enums are not aligned so translate them manually.
>
>     Also simply return STATUS_NOT_SUPPORTED when an invalid Hash->Algorithm
>     is supplied.
>
>     Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
>     ---
>       src/xenvif/frontend.c | 11 ++++++-----
>       1 file changed, 6 insertions(+), 5 deletions(-)
>
>     diff --git a/src/xenvif/frontend.c b/src/xenvif/frontend.c
>     index 79b04fb..86bebf8 100644
>     --- a/src/xenvif/frontend.c
>     +++ b/src/xenvif/frontend.c
>     @@ -1883,6 +1883,7 @@ __FrontendUpdateHash(
>       {
>           PXENVIF_CONTROLLER      Controller;
>           ULONG                   Zero = 0;
>     +    ULONG                   NetifAlgorithm;
>           ULONG                   Size;
>           PULONG                  Mapping;
>           ULONG                   Flags;
>     @@ -1892,12 +1893,14 @@ __FrontendUpdateHash(
>
>           switch (Hash->Algorithm) {
>           case XENVIF_PACKET_HASH_ALGORITHM_NONE:
>     +        NetifAlgorithm = XEN_NETIF_CTRL_HASH_ALGORITHM_NONE;
>               Size = 1;
>               Mapping = &Zero;
>               Flags = 0;
>               break;
>
>           case XENVIF_PACKET_HASH_ALGORITHM_TOEPLITZ:
>     +        NetifAlgorithm = XEN_NETIF_CTRL_HASH_ALGORITHM_TOEPLITZ;
>               Size = Hash->Size;
>               Mapping = Hash->Mapping;
>               Flags = Hash->Flags;
>     @@ -1905,17 +1908,15 @@ __FrontendUpdateHash(
>
>           case XENVIF_PACKET_HASH_ALGORITHM_UNSPECIFIED:
>           default:
>     -        (VOID) ControllerSetHashAlgorithm(Controller,
>     -
>     XEN_NETIF_CTRL_HASH_ALGORITHM_NONE);
>     -        goto done;
>     +        return STATUS_NOT_SUPPORTED;
>           }
>
>           status = ControllerSetHashAlgorithm(Controller,
>     -                                        Hash->Algorithm);
>     +                                        NetifAlgorithm);
>           if (!NT_SUCCESS(status))
>               goto fail1;
>
>     -    if (Hash->Algorithm == XEN_NETIF_CTRL_HASH_ALGORITHM_NONE)
>     +    if (NetifAlgorithm == XEN_NETIF_CTRL_HASH_ALGORITHM_NONE)
>               goto done;
>
>           status = ControllerSetHashMappingSize(Controller, Size);
>     --
>     2.50.1.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