[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XENVIF PATCH] Fix argument to ControllerSetHashAlgorithm
On 23/07/2025 10:53, Owen Smith wrote: > From what I can tell, xennet will only set NONE or TOEPLITZ. But xenvif > defaults the Hash->Algorithm to UNSPECIFIED, so its possible that xennet > may not set the algorithm and leave it as default. > UNSPECIFIED seems to only be used as a log line to detect when xennet > has not set an algorithm. > Either UNSPECIFIED should be treated the same as NONE, or removed > entirely - though removal would require a change to the interface > header, and a version change... > > switch (Hash->Algorithm) { > case XENVIF_PACKET_HASH_ALGORITHM_NONE: > case XENVIF_PACKET_HASH_ALGORITHM_UNSPECIFIED: > NetifAlgorithm = XEN_NETIF_CTRL_HASH_ALGORITHM_NONE; > Size = 1; > Mapping = &Zero; > Flags = 0; > break; > > adding UNSPECIFIED to this switch in __FrontendUpdateHash is likely the > simplest solution. > At this point, Size, Mapping and Flags dont need to be assigned when > NetifAlgorithm is NONE, as they are not used due to the 'goto done' code > path > > Owen That sounds good to me, it also tracks with the other places that handle XENVIF_PACKET_HASH_ALGORITHM. I'll prepare a v2. > > On Wed, Jul 23, 2025 at 9:38 AM Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx> wrote: > > 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> <https:// > vates.tech <https://vates.tech>> > > > > > > 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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |