* [PATCH V2 0/5] Adding necessary changes for RedfishPkg
@ 2023-05-04 4:19 Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Minh Nguyen @ 2023-05-04 4:19 UTC (permalink / raw)
To: devel; +Cc: patches, abner.chang, nicklew, igork, nhi, Minh Nguyen
This patchset adds necessary changes for RedfishPkg to avoid some unexpected cases and fix compilation.
Changes since v2:
+ Add "Cc:" to maintainers in commit message.
+ Correct patch format subject prefix.
Changes since v1:
+ Remove redundant change and change the commit message for more precise in PATCH 2/5.
Nhi Pham (1):
RedfishPkg: Add missing newline character
Vu Nguyen (4):
RedfishPkg: Correct variable type to prevent memory corruption
RedfishPkg: Fix condition checking of error status
RedfishPkg: Create RestEx child on selected interface
RedfishPkg: Fix compile issue on Linux
RedfishPkg/Include/Library/RestExLib.h | 3 +
RedfishPkg/Include/Pcd/RestExServiceDevicePath.h | 5 +-
RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h | 3 +-
RedfishPkg/Library/DxeRestExLib/DxeRestExLib.c | 153 ++++++++------------
RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 4 +-
RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c | 3 +-
6 files changed, 73 insertions(+), 98 deletions(-)
--
2.39.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V2 1/5] RedfishPkg: Correct variable type to prevent memory corruption
2023-05-04 4:19 [PATCH V2 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
@ 2023-05-04 4:19 ` Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 2/5] RedfishPkg: Fix condition checking of error status Minh Nguyen
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Minh Nguyen @ 2023-05-04 4:19 UTC (permalink / raw)
To: devel
Cc: patches, abner.chang, nicklew, igork, nhi, Vu Nguyen, Minh Nguyen,
Nick Ramirez
From: Vu Nguyen <vunguyen@os.amperecomputing.com>
Id will be casted by CoreOpenProtocol, declare this variable with a
wrong type might result in the corruption of other local variables.
Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nick Ramirez <nramirez@nvidia.com>
Reviewed-by: Nickle Wang <nicklew@nvidia.com>
---
RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
index 4a9e9f7b81a4..6b94e5814c43 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
@@ -3,6 +3,7 @@
Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
(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
@@ -84,7 +85,7 @@ struct _RESTEX_SERVICE {
//
EFI_HANDLE HttpChildHandle;
- UINT32 Id;
+ UINT32 *Id;
EFI_REST_EX_SERVICE_INFO RestExServiceInfo;
};
--
2.39.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 2/5] RedfishPkg: Fix condition checking of error status
2023-05-04 4:19 [PATCH V2 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
@ 2023-05-04 4:19 ` Minh Nguyen
2023-05-04 7:00 ` Nickle Wang
2023-05-04 4:19 ` [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface Minh Nguyen
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Minh Nguyen @ 2023-05-04 4:19 UTC (permalink / raw)
To: devel
Cc: patches, abner.chang, nicklew, igork, nhi, Vu Nguyen, Minh Nguyen,
Nick Ramirez
From: Vu Nguyen <vunguyen@os.amperecomputing.com>
This change fixes condition checking of error status, the condition
should be compared with TRUE status to be identical with an error message.
Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nick Ramirez <nramirez@nvidia.com>
---
RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 583c6f78e19c..a247bfed9e34 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -5,6 +5,7 @@
(C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
Copyright (c) 2022, AMD Incorporated. All rights reserved.
Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -926,7 +927,7 @@ AddAndSignalNewRedfishService (
}
Status = gBS->SignalEvent (Instance->DiscoverToken->Event);
- if (!EFI_ERROR (Status)) {
+ if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "%a:No event to signal!\n", __func__));
}
}
--
2.39.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface
2023-05-04 4:19 [PATCH V2 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 2/5] RedfishPkg: Fix condition checking of error status Minh Nguyen
@ 2023-05-04 4:19 ` Minh Nguyen
2023-05-04 8:00 ` Chang, Abner
2023-05-04 4:19 ` [PATCH V2 4/5] RedfishPkg: Add missing newline character Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 5/5] RedfishPkg: Fix compile issue on Linux Minh Nguyen
4 siblings, 1 reply; 11+ messages in thread
From: Minh Nguyen @ 2023-05-04 4:19 UTC (permalink / raw)
To: devel
Cc: patches, abner.chang, nicklew, igork, nhi, Vu Nguyen, Minh Nguyen,
Nick Ramirez
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 <minhnguyen1@os.amperecomputing.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nick Ramirez <nramirez@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__,
+ 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 4/5] RedfishPkg: Add missing newline character
2023-05-04 4:19 [PATCH V2 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
` (2 preceding siblings ...)
2023-05-04 4:19 ` [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface Minh Nguyen
@ 2023-05-04 4:19 ` Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 5/5] RedfishPkg: Fix compile issue on Linux Minh Nguyen
4 siblings, 0 replies; 11+ messages in thread
From: Minh Nguyen @ 2023-05-04 4:19 UTC (permalink / raw)
To: devel; +Cc: patches, abner.chang, nicklew, igork, nhi, Minh Nguyen,
Nick Ramirez
From: Nhi Pham <nhi@os.amperecomputing.com>
This adds a missing newline character to make the error log
readable in case the module is failed.
Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nick Ramirez <nramirez@nvidia.com>
Reviewed-by: Nickle Wang <nicklew@nvidia.com>
---
RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
index 45fc6e2182bc..c4fa59193a2f 100644
--- a/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
+++ b/RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.c
@@ -8,6 +8,7 @@
(C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
Copyright (C) 2022 Advanced Micro Devices, Inc. All rights reserved.<BR>
Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -59,7 +60,7 @@ RedfishCreateSmbiosTable42 (
Status = RedfishPlatformHostInterfaceDeviceDescriptor (&DeviceType, &DeviceDescriptor);
if (EFI_ERROR (Status)) {
if (Status == EFI_NOT_FOUND) {
- DEBUG ((DEBUG_ERROR, "%a: No Redfish host interface descriptor is provided on this platform.", __func__));
+ DEBUG ((DEBUG_ERROR, "%a: No Redfish host interface descriptor is provided on this platform.\n", __func__));
return EFI_NOT_FOUND;
}
--
2.39.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V2 5/5] RedfishPkg: Fix compile issue on Linux
2023-05-04 4:19 [PATCH V2 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
` (3 preceding siblings ...)
2023-05-04 4:19 ` [PATCH V2 4/5] RedfishPkg: Add missing newline character Minh Nguyen
@ 2023-05-04 4:19 ` Minh Nguyen
4 siblings, 0 replies; 11+ messages in thread
From: Minh Nguyen @ 2023-05-04 4:19 UTC (permalink / raw)
To: devel
Cc: patches, abner.chang, nicklew, igork, nhi, Vu Nguyen, Minh Nguyen,
Nick Ramirez, Abner Chang
From: Vu Nguyen <vunguyen@os.amperecomputing.com>
It requires a fixed size array to store the content of device path PCD.
Add the array size to solve this issue.
Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nick Ramirez <nramirez@nvidia.com>
Reviewed-by: Abner Chang <Abner.Chang@amd.com>
---
RedfishPkg/Include/Pcd/RestExServiceDevicePath.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h b/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
index 91b1198297c2..57fc199f61f2 100644
--- a/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
+++ b/RedfishPkg/Include/Pcd/RestExServiceDevicePath.h
@@ -4,6 +4,7 @@
Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
(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
@@ -14,6 +15,8 @@
#include <Protocol/DevicePath.h>
+#define MAX_DEVICE_PATH_NODE 40
+
typedef enum {
DEVICE_PATH_MATCH_MAC_NODE = 1,
DEVICE_PATH_MATCH_PCI_NODE = 2,
@@ -32,7 +35,7 @@ typedef struct {
// 0x03,0x0b,0x25,0x00,0x00,0x50,0x56,0xc0,0x00,0x02,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01,
// 0x7f,0xff,0x04,0x00}
//
- EFI_DEVICE_PATH_PROTOCOL DevicePath[];
+ EFI_DEVICE_PATH_PROTOCOL DevicePath[MAX_DEVICE_PATH_NODE];
} REST_EX_SERVICE_DEVICE_PATH_DATA;
#endif
--
2.39.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V2 2/5] RedfishPkg: Fix condition checking of error status
2023-05-04 4:19 ` [PATCH V2 2/5] RedfishPkg: Fix condition checking of error status Minh Nguyen
@ 2023-05-04 7:00 ` Nickle Wang
0 siblings, 0 replies; 11+ messages in thread
From: Nickle Wang @ 2023-05-04 7:00 UTC (permalink / raw)
To: Minh Nguyen, devel@edk2.groups.io
Cc: patches@amperecomputing.com, abner.chang@amd.com, igork@ami.com,
nhi@os.amperecomputing.com, Vu Nguyen, Nick Ramirez
Thanks for fixing this issue.
Reviewed-by: Nickle Wang <nicklew@nvidia.com>
Regards,
Nickle
> -----Original Message-----
> From: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
> Sent: Thursday, May 4, 2023 12:20 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; abner.chang@amd.com; Nickle Wang
> <nicklew@nvidia.com>; igork@ami.com; nhi@os.amperecomputing.com; Vu
> Nguyen <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen1@os.amperecomputing.com>; Nick Ramirez
> <nramirez@nvidia.com>
> Subject: [PATCH V2 2/5] RedfishPkg: Fix condition checking of error status
>
> External email: Use caution opening links or attachments
>
>
> From: Vu Nguyen <vunguyen@os.amperecomputing.com>
>
> This change fixes condition checking of error status, the condition should be
> compared with TRUE status to be identical with an error message.
>
> Signed-off-by: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nick Ramirez <nramirez@nvidia.com>
> ---
> RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 583c6f78e19c..a247bfed9e34 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -5,6 +5,7 @@
> (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
> Copyright (c) 2022, AMD Incorporated. All rights reserved.
> Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> + Copyright (c) 2023, Ampere Computing LLC. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -926,7 +927,7 @@ AddAndSignalNewRedfishService (
> }
>
> Status = gBS->SignalEvent (Instance->DiscoverToken->Event);
> - if (!EFI_ERROR (Status)) {
> + if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "%a:No event to signal!\n", __func__));
> }
> }
> --
> 2.39.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface
2023-05-04 4:19 ` [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface Minh Nguyen
@ 2023-05-04 8:00 ` Chang, Abner
2023-05-04 8:26 ` Minh Nguyen
0 siblings, 1 reply; 11+ messages in thread
From: Chang, Abner @ 2023-05-04 8:00 UTC (permalink / raw)
To: Minh Nguyen, devel@edk2.groups.io
Cc: patches@amperecomputing.com, nicklew@nvidia.com, igork@ami.com,
nhi@os.amperecomputing.com, Vu Nguyen, Nick Ramirez
[AMD Official Use Only - General]
Hi Minh, please check my comment inline,
> -----Original Message-----
> From: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
> Sent: Thursday, May 4, 2023 12:20 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; Vu Nguyen
> <vunguyen@os.amperecomputing.com>; Minh Nguyen
> <minhnguyen1@os.amperecomputing.com>; Nick Ramirez
> <nramirez@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 <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 <minhnguyen1@os.amperecomputing.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nick Ramirez <nramirez@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface
2023-05-04 8:00 ` Chang, Abner
@ 2023-05-04 8:26 ` Minh Nguyen
2023-05-05 3:59 ` Minh Nguyen
0 siblings, 1 reply; 11+ messages in thread
From: Minh Nguyen @ 2023-05-04 8:26 UTC (permalink / raw)
To: Chang, Abner, Minh Nguyen, devel@edk2.groups.io
Cc: patches@amperecomputing.com, nicklew@nvidia.com, igork@ami.com,
nhi@os.amperecomputing.com, Vu Nguyen, Nick Ramirez
Hi Abner,
Let me check in my system and will give the confirmation to you.
Thanks,
Minh Nguyen
On 5/4/2023 3:00 PM, Chang, Abner wrote:
> [AMD Official Use Only - General]
>
> Hi Minh, please check my comment inline,
>
>> -----Original Message-----
>> From: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
>> Sent: Thursday, May 4, 2023 12:20 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; Vu Nguyen
>> <vunguyen@os.amperecomputing.com>; Minh Nguyen
>> <minhnguyen1@os.amperecomputing.com>; Nick Ramirez
>> <nramirez@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 <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 <minhnguyen1@os.amperecomputing.com>
>> Cc: Abner Chang <abner.chang@amd.com>
>> Cc: Igor Kulchytskyy <igork@ami.com>
>> Cc: Nick Ramirez <nramirez@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface
2023-05-04 8:26 ` Minh Nguyen
@ 2023-05-05 3:59 ` Minh Nguyen
2023-05-05 4:03 ` Chang, Abner
0 siblings, 1 reply; 11+ messages in thread
From: Minh Nguyen @ 2023-05-05 3:59 UTC (permalink / raw)
To: Chang, Abner, Minh Nguyen, devel@edk2.groups.io
Cc: patches@amperecomputing.com, nicklew@nvidia.com, igork@ami.com,
nhi@os.amperecomputing.com, Vu Nguyen, Nick Ramirez
Hi Abner,
I have checked in my system and it works fines when using
RedfishConfigHandlerDriver.
I also updated new patches, could you please help me to check it?
Thanks,
Minh Nguyen
On 5/4/2023 3:26 PM, Minh Nguyen wrote:
> Hi Abner,
>
> Let me check in my system and will give the confirmation to you.
>
>
> Thanks,
>
> Minh Nguyen
>
> On 5/4/2023 3:00 PM, Chang, Abner wrote:
>> [AMD Official Use Only - General]
>>
>> Hi Minh, please check my comment inline,
>>
>>> -----Original Message-----
>>> From: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
>>> Sent: Thursday, May 4, 2023 12:20 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; Vu Nguyen
>>> <vunguyen@os.amperecomputing.com>; Minh Nguyen
>>> <minhnguyen1@os.amperecomputing.com>; Nick Ramirez
>>> <nramirez@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 <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 <minhnguyen1@os.amperecomputing.com>
>>> Cc: Abner Chang <abner.chang@amd.com>
>>> Cc: Igor Kulchytskyy <igork@ami.com>
>>> Cc: Nick Ramirez <nramirez@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface
2023-05-05 3:59 ` Minh Nguyen
@ 2023-05-05 4:03 ` Chang, Abner
0 siblings, 0 replies; 11+ messages in thread
From: Chang, Abner @ 2023-05-05 4:03 UTC (permalink / raw)
To: Minh Nguyen, Minh Nguyen, devel@edk2.groups.io
Cc: patches@amperecomputing.com, nicklew@nvidia.com, igork@ami.com,
nhi@os.amperecomputing.com, Vu Nguyen, Nick Ramirez
[-- Attachment #1: Type: text/plain, Size: 13566 bytes --]
Ok, I will check.
Please just go ahead to create a PR against edk2 repo. It will then trigger CI. We can merge it to master today if there is no problem as soft freeze is coming soon.
Thanks
Abner
Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: Minh Nguyen <minhnguyen1@amperemail.onmicrosoft.com>
Sent: Friday, May 5, 2023, 11:59 AM
To: Chang, Abner <Abner.Chang@amd.com>; Minh Nguyen <minhnguyen1@os.amperecomputing.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: patches@amperecomputing.com <patches@amperecomputing.com>; nicklew@nvidia.com <nicklew@nvidia.com>; igork@ami.com <igork@ami.com>; nhi@os.amperecomputing.com <nhi@os.amperecomputing.com>; Vu Nguyen <vunguyen@os.amperecomputing.com>; Nick Ramirez <nramirez@nvidia.com>
Subject: Re: [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.
Hi Abner,
I have checked in my system and it works fines when using
RedfishConfigHandlerDriver.
I also updated new patches, could you please help me to check it?
Thanks,
Minh Nguyen
On 5/4/2023 3:26 PM, Minh Nguyen wrote:
> Hi Abner,
>
> Let me check in my system and will give the confirmation to you.
>
>
> Thanks,
>
> Minh Nguyen
>
> On 5/4/2023 3:00 PM, Chang, Abner wrote:
>> [AMD Official Use Only - General]
>>
>> Hi Minh, please check my comment inline,
>>
>>> -----Original Message-----
>>> From: Minh Nguyen <minhnguyen1@os.amperecomputing.com>
>>> Sent: Thursday, May 4, 2023 12:20 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; Vu Nguyen
>>> <vunguyen@os.amperecomputing.com>; Minh Nguyen
>>> <minhnguyen1@os.amperecomputing.com>; Nick Ramirez
>>> <nramirez@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 <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 <minhnguyen1@os.amperecomputing.com>
>>> Cc: Abner Chang <abner.chang@amd.com>
>>> Cc: Igor Kulchytskyy <igork@ami.com>
>>> Cc: Nick Ramirez <nramirez@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
[-- Attachment #2: Type: text/html, Size: 27332 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-05 4:03 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-04 4:19 [PATCH V2 0/5] Adding necessary changes for RedfishPkg Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 1/5] RedfishPkg: Correct variable type to prevent memory corruption Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 2/5] RedfishPkg: Fix condition checking of error status Minh Nguyen
2023-05-04 7:00 ` Nickle Wang
2023-05-04 4:19 ` [PATCH V2 3/5] RedfishPkg: Create RestEx child on selected interface Minh Nguyen
2023-05-04 8:00 ` Chang, Abner
2023-05-04 8:26 ` Minh Nguyen
2023-05-05 3:59 ` Minh Nguyen
2023-05-05 4:03 ` Chang, Abner
2023-05-04 4:19 ` [PATCH V2 4/5] RedfishPkg: Add missing newline character Minh Nguyen
2023-05-04 4:19 ` [PATCH V2 5/5] RedfishPkg: Fix compile issue on Linux Minh Nguyen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox