[AMD Official Use Only - General]
Hi Minh, please check my comment inline,
-----Original Message-----
From: Minh Nguyen <minhnguy...@os.amperecomputing.com>
Sent: Thursday, May 4, 2023 12:20 PM
To: devel@edk2.groups.io
Cc: patc...@amperecomputing.com; Chang, Abner
<abner.ch...@amd.com>; nick...@nvidia.com; ig...@ami.com;
n...@os.amperecomputing.com; Vu Nguyen
<vungu...@os.amperecomputing.com>; Minh Nguyen
<minhnguy...@os.amperecomputing.com>; Nick Ramirez
<nrami...@nvidia.com>
Subject: [PATCH V2 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 <vungu...@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 <minhnguy...@os.amperecomputing.com>
Cc: Abner Chang <abner.ch...@amd.com>
Cc: Igor Kulchytskyy <ig...@ami.com>
Cc: Nick Ramirez <nrami...@nvidia.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__,
The latest PatchCheck throws error for "__FUNCTION__", we should use "__func__"
otherwise it wont it pass PatchCheck and edk2 CI.
I have no problem with this code in generally. Just would like to confirm with
you if you use RedfishConfigHandlerDriver on your system and does it work fine
with the change in this patch?
Thanks
Abner
+ 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 a247bfed9e34..dd5bdbb377e9 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
