Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface

[Minh Nguyen OS <minhnguyen@os.amperecomputing.com>]

[-- Attachment #1: Type: text/plain, Size: 13588 bytes --]

Hi Chang,

 Yes, this is the first time and the issue comes from my internal mail and waiting for fixing. Thanks for that information and will check with miki.demeter@intel.com as well.

Thanks,
Minh Nguyen.
________________________________
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Thursday, April 20, 2023 8:28 AM
To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface


[AMD Official Use Only - General]

Hi Minh,
Is this the first time you send email to groups.io, if yes please also check it with Miki
miki.demeter@intel.com to confirm is your mail passed through groups.io or not.

Thanks
Abner


Get Outlook for Android<https://aka.ms/AAb9ysg>

________________________________
From: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>
Sent: Thursday, April 20, 2023, 9:23 AM
To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: Re: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Hi Chang,

  It seems like my email has the problem, please expect the delay for my v2 patches while my email's fixing. Thanks for your patience.

Thanks,
Minh Nguyen
________________________________
From: Chang, Abner <Abner.Chang@amd.com>
Sent: Sunday, April 16, 2023 11:36 AM
To: Minh Nguyen OS <minhnguyen@os.amperecomputing.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Open Source Submission <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; Nhi Pham OS <nhi@os.amperecomputing.com>; Tinh Nguyen OS <tinhnguyen@os.amperecomputing.com>; Vu Nguyen OS <vunguyen@os.amperecomputing.com>
Subject: RE: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface

[AMD Official Use Only - General]

Hi Minh,
That is weird I can't find this patch set on group.io, and I can't apply this patch from email neither. Could you please check it?

Thanks
Abner

> -----Original Message-----
> From: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> Sent: Friday, April 14, 2023 4:19 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; Chang, Abner
> <Abner.Chang@amd.com>; nicklew@nvidia.com; igork@ami.com;
> nhi@os.amperecomputing.com; tinhnguyen@os.amperecomputing.com; Vu
> Nguyen <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen@os.amperecomputing.com>
> Subject: [PATCH 3/5] RedfishPkg: Create RestEx child on selected interface
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
>
> When a MAC address matching interface is found, a RestEx child will be
> created to provide the Redfish communication on that interface.
> Currently, It will try to locate all RestEx binding services and choose the first
> satisfied instance without taking care about current selected interface. This
> might raise an issue on the system with multiple network devices that the
> RestEx child was installed on wrong interface.
>
> Signed-off-by: Minh Nguyen <minhnguyen@os.amperecomputing.com>
> ---
>  RedfishPkg/Include/Library/RestExLib.h             |   3 +
>  RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c     | 153 ++++++++----------
> --
>  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c |   1 +
>  3 files changed, 63 insertions(+), 94 deletions(-)
>
> diff --git a/RedfishPkg/Include/Library/RestExLib.h
> b/RedfishPkg/Include/Library/RestExLib.h
> index bc4e4ca6caa7..2c32c3684cf4 100644
> --- a/RedfishPkg/Include/Library/RestExLib.h
> +++ b/RedfishPkg/Include/Library/RestExLib.h
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -20,6 +21,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
>
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -32,6 +34,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> diff --git a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> index d9acad24dec1..6002be93c54e 100644
> --- a/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> +++ b/RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c
> @@ -2,6 +2,7 @@
>    This library provides help functions for REST EX Protocol.
>
>    (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -9,6 +10,7 @@
>
>  #include <Uefi.h>
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>  #include <Library/NetLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> @@ -21,6 +23,7 @@
>    This function allows the caller to create child handle for specific
>    REST server.
>
> +  @param[in]  Controller           The controller handle used of selected
> interface.
>    @param[in]  Image                The image handle used to open service.
>    @param[in]  AccessMode           Access mode of REST server.
>    @param[in]  ConfigType           Underlying configuration to communicate
> with REST server.
> @@ -33,6 +36,7 @@
>  **/
>  EFI_STATUS
>  RestExLibCreateChild (
> +  IN EFI_HANDLE                       Controller,
>    IN EFI_HANDLE                       Image,
>    IN EFI_REST_EX_SERVICE_ACCESS_MODE  AccessMode,
>    IN EFI_REST_EX_CONFIG_TYPE          ConfigType,
> @@ -41,8 +45,6 @@ RestExLibCreateChild (
>    )
>  {
>    EFI_STATUS                Status;
> -  UINTN                     NoBuffer;
> -  EFI_HANDLE                *Handle;
>    EFI_HANDLE                ChildHandle;
>    EFI_REST_EX_PROTOCOL      *RestEx;
>    EFI_REST_EX_SERVICE_INFO  *RestExServiceInfo; @@ -59,116 +61,79 @@
> RestExLibCreateChild (
>    }
>
>    *ChildInstanceHandle = NULL;
> -  //
> -  // Locate all REST EX binding service.
> -  //
> -  Handle   = NULL;
> -  NoBuffer = 0;
> -  Status   = gBS->LocateHandleBuffer (
> -                    ByProtocol,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    NULL,
> -                    &NoBuffer,
> -                    &Handle
> -                    );
> -  if (EFI_ERROR (Status) && (Status != EFI_BUFFER_TOO_SMALL)) {
> +
> +  ChildHandle = NULL;
> +  Status = NetLibCreateServiceChild (
> +             Controller,
> +             Image,
> +             &gEfiRestExServiceBindingProtocolGuid,
> +             &ChildHandle
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Failed to create service child - %r \n",
> +      __FUNCTION__,
> +      Status
> +      ));
>      return Status;
>    }
> -
> -  Handle = (EFI_HANDLE *)AllocateZeroPool (sizeof (EFI_HANDLE) *
> NoBuffer);
> -  if (Handle == NULL) {
> -    return EFI_OUT_OF_RESOURCES;
> -  }
> -
> -  Status = gBS->LocateHandleBuffer (
> -                  ByProtocol,
> -                  &gEfiRestExServiceBindingProtocolGuid,
> +  Status = gBS->OpenProtocol (
> +                  ChildHandle,
> +                  &gEfiRestExProtocolGuid,
> +                  (VOID **)&RestEx,
> +                  Image,
>                    NULL,
> -                  &NoBuffer,
> -                  &Handle
> +                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
>                    );
>    if (EFI_ERROR (Status)) {
> -    FreePool (Handle);
> -    return Status;
> +    goto ON_ERROR;
>    }
>
>    //
> -  // Search for the proper REST EX instance.
> +  // Get the information of REST service provided by this EFI REST EX
> + driver
>    //
> -  while (NoBuffer != 0) {
> -    ChildHandle = NULL;
> -    Status      = NetLibCreateServiceChild (
> -                    *(Handle + (NoBuffer - 1)),
> -                    Image,
> -                    &gEfiRestExServiceBindingProtocolGuid,
> -                    &ChildHandle
> -                    );
> -    if (!EFI_ERROR (Status)) {
> -      Status = gBS->OpenProtocol (
> -                      ChildHandle,
> -                      &gEfiRestExProtocolGuid,
> -                      (VOID **)&RestEx,
> -                      Image,
> -                      NULL,
> -                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
> -                      );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Get the information of REST service provided by this EFI REST EX driver
> -      //
> -      Status = RestEx->GetService (
> -                         RestEx,
> -                         &RestExServiceInfo
> -                         );
> -      if (EFI_ERROR (Status)) {
> -        goto ON_ERROR;
> -      }
> -
> -      //
> -      // Check REST EX property.
> -      //
> -      switch (ConfigType) {
> -        case EfiRestExConfigHttp:
> -          LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> -          break;
> -
> -        case EfiRestExConfigUnspecific:
> -          LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> -          break;
> +  Status = RestEx->GetService (
> +                     RestEx,
> +                     &RestExServiceInfo
> +                     );
> +  if (EFI_ERROR (Status)) {
> +    goto ON_ERROR;
> +  }
> +  //
> +  // Check REST EX property.
> +  //
> +  switch (ConfigType) {
> +  case EfiRestExConfigHttp:
> +    LenOfConfig = sizeof (EFI_REST_EX_HTTP_CONFIG_DATA);
> +    break;
>
> -        default:
> -          goto ON_ERROR;
> -      }
> +  case EfiRestExConfigUnspecific:
> +    LenOfConfig = REST_EX_CONFIG_DATA_LEN_UNKNOWN;
> +    break;
>
> -      if ((RestExServiceInfo-
> >EfiRestExServiceInfoV10.RestServiceAccessMode != AccessMode) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType) ||
> -          (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType) ||
> -          ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig)))
> -      {
> -        goto ON_ERROR;
> -      }
> -    }
> +  default:
> +    goto ON_ERROR;
> +  }
> +  if (RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceAccessMode !=
> AccessMode ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestServiceType !=
> ServiceType ||
> +      RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigType !=
> ConfigType ||
> +      ((LenOfConfig != REST_EX_CONFIG_DATA_LEN_UNKNOWN) &&
> (RestExServiceInfo->EfiRestExServiceInfoV10.RestExConfigDataLength !=
> LenOfConfig))) {
> +    goto ON_ERROR;
> +  }
>
> -    //
> -    // This is proper REST EX instance.
> -    //
> -    *ChildInstanceHandle = ChildHandle;
> -    FreePool (Handle);
> -    return EFI_SUCCESS;
> +  //
> +  // This is proper REST EX instance.
> +  //
> +  *ChildInstanceHandle = ChildHandle;
> +  return EFI_SUCCESS;
>
>  ON_ERROR:;
>      NetLibDestroyServiceChild (
> -      *(Handle + (NoBuffer - 1)),
> +      Controller,
>        Image,
>        &gEfiRestExServiceBindingProtocolGuid,
>        ChildHandle
>        );
> -    NoBuffer--;
> -  }
> -
> -  FreePool (Handle);
>    return EFI_NOT_FOUND;
>  }
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 29e7b0624ebf..86ec88ddafc4 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -87,6 +87,7 @@ CreateRestExInstance (
>    EFI_STATUS  Status;
>
>    Status = RestExLibCreateChild (
> +             Instance->NetworkInterface->OpenDriverControllerHandle,
>               Instance->Owner,
>               FixedPcdGetBool (PcdRedfishDiscoverAccessModeInBand) ?
> EfiRestExServiceInBandAccess : EfiRestExServiceOutOfBandAccess,
>               EfiRestExConfigHttp,
> --
> 2.39.0


[-- Attachment #2: Type: text/html, Size: 29789 bytes --]