From: "Nickle Wang" <nicklew@nvidia.com>
To: "Chang, Abner" <Abner.Chang@amd.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Igor Kulchytskyy <igork@ami.com>
Subject: Re: [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add Redfish Resource Addendum Library
Date: Mon, 15 May 2023 07:39:27 +0000 [thread overview]
Message-ID: <MW4PR12MB7031CDC04C1614A7B331E5CFD9789@MW4PR12MB7031.namprd12.prod.outlook.com> (raw)
In-Reply-To: <MN2PR12MB3966948709CBD9170EF11180EA789@MN2PR12MB3966.namprd12.prod.outlook.com>
Thanks for your review comments, Abner! Updated in version 2 patch-set.
Regards,
Nickle
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: Monday, May 15, 2023 11:20 AM
> To: Nickle Wang <nicklew@nvidia.com>; devel@edk2.groups.io
> Cc: Igor Kulchytskyy <igork@ami.com>
> Subject: RE: [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add Redfish
> Resource Addendum Library
>
> External email: Use caution opening links or attachments
>
>
> [AMD Official Use Only - General]
>
> Hi Nickle,
> I read though this patch and was confused by function naming and the function
> description in the function header. I think we have to revise it and make the
> description of functionality clear for user to follow.
>
> I apologize if we ever discussed this before however I don't have the synchronous
> mind now.
>
> 1. Protocol definitions
> This protocol is provided by platform that would like to add an additional Redfish
> resource in "Oem" property or to add other platform specific property in the
> name other than "Oem".
> Both of two protocol interfaces give platform an chance to add "OEM" or other
> Redfish property, however the naming of function prototypes are not consistent.
> The naming of "OemCallback" and "ProvisioningCallback" are also not consistent.
>
> How do you think the naming below,
>
> struct _EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL {
> UINT64 Revision;
> EDKII_REDFISH_RESOURCE_ADDENDUM_OEM
> AddAddendumOemRedfishResource; ///<
> EDKII_REDFISH_RESOURCE_ADDENDUM_OTHER
> AddAddendumOtherRedfishResource; ///<
> };
> Would above look simple and clear? Also, please add some comments to these
> two functions if you are agree with this change.
>
> 2. Function naming in the library,
> RedfishGetOemData -> RedfishGetAddendumOemData
> RedfishGetAddendumData-> RedfishGetAddendumOtherData
>
> 3. The current description in function header looks to me not quite clear. Please
> elaborates some more functionality in the function header and parameters
>
> 4. Shall we have to update readme.md?
>
> Thanks
> Abner
>
>
> > -----Original Message-----
> > From: Nickle Wang <nicklew@nvidia.com>
> > Sent: Friday, May 12, 2023 3:23 PM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner <Abner.Chang@amd.com>; Igor Kulchytskyy
> > <igork@ami.com>
> > Subject: [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add
> > Redfish Resource Addendum Library
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Implement RedfishAddendumLib to support Redfish Resource Addendum
> > Protocol.
> > Feature driver calls this library to get addendum data before
> > providing data to Redfish service
> >
> > Signed-off-by: Nickle Wang <nicklew@nvidia.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > ---
> > RedfishClientPkg/RedfishClientLibs.dsc.inc | 4 +-
> > RedfishClientPkg/RedfishClientPkg.dsc | 2 +
> > .../RedfishAddendumLib/RedfishAddendumLib.inf | 40 +++
> > .../Include/Library/RedfishAddendumLib.h | 65 +++++
> > .../RedfishAddendumLib/RedfishAddendumLib.c | 262
> > ++++++++++++++++++
> > 5 files changed, 372 insertions(+), 1 deletion(-) create mode 100644
> > RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> > create mode 100644
> > RedfishClientPkg/Include/Library/RedfishAddendumLib.h
> > create mode 100644
> > RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c
> >
> > diff --git a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > index fe0c4b08..5ea38015 100644
> > --- a/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > +++ b/RedfishClientPkg/RedfishClientLibs.dsc.inc
> > @@ -2,10 +2,11 @@
> > # Redfish DSC include file for [LibraryClasses*] section of all Architectures.
> > #
> > # This file can be included to the [LibraryClasses*] section(s) of a
> > platform DSC file -# by using "!include RedfishPkg/RedfisLibs.dsc.inc"
> > to specify the library instances
> > +# by using "!include RedfishPkg/RedfishLibs.dsc.inc" to specify the
> > +library
> > instances
> > # of EDKII network library classes.
> > #
> > # (C) Copyright 2021-2022 Hewlett Packard Enterprise Development
> > LP<BR>
> > +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> > reserved.
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > @@ -37,3 +38,4 @@
> >
> > EdkIIRedfishResourceConfigLib|RedfishClientPkg/Library/EdkIIRedfishRes
> > EdkIIRedfishResourceConfigLib|ou
> > rceConfigLib/EdkIIRedfishResourceConfigLib.inf
> >
> > RedfishEventLib|RedfishClientPkg/Library/RedfishEventLib/RedfishEventL
> > RedfishEventLib|ib.i
> > nf
> >
> > RedfishVersionLib|RedfishClientPkg/Library/RedfishVersionLib/RedfishVe
> > RedfishVersionLib|rsi
> > onLib.inf
> > +
> > RedfishAddendumLib|RedfishClientPkg/Library/RedfishAddendumLib/Redfi
> > shAddendumLib.inf
> > diff --git a/RedfishClientPkg/RedfishClientPkg.dsc
> > b/RedfishClientPkg/RedfishClientPkg.dsc
> > index 2b2149cc..d3b645b6 100644
> > --- a/RedfishClientPkg/RedfishClientPkg.dsc
> > +++ b/RedfishClientPkg/RedfishClientPkg.dsc
> > @@ -2,6 +2,7 @@
> > # Redfish Client Package
> > #
> > # (C) Copyright 2021 Hewlett-Packard Enterprise Development LP.
> > +# Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights
> > reserved.
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > @@ -49,5 +50,6 @@
> >
> >
> > RedfishClientPkg/Library/RedfishFeatureUtilityLib/RedfishFeatureUtilit
> > yLib.i
> > nf
> > RedfishClientPkg/PrivateLibrary/RedfishLib/RedfishLib.inf
> > + RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> >
> > !include RedfishClientPkg/RedfishClient.dsc.inc
> > diff --git
> > a/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> > b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> > new file mode 100644
> > index 00000000..1ecfaa69
> > --- /dev/null
> > +++
> > b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.inf
> > @@ -0,0 +1,40 @@
> > +## @file
> > +#
> > +# Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All
> > +rights
> > reserved.
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> > +
> > +[Defines]
> > + INF_VERSION = 0x00010006
> > + BASE_NAME = RedfishAddendumLib
> > + FILE_GUID = 7B227D39-746D-4247-8291-27B0FA79A7AF
> > + MODULE_TYPE = DXE_DRIVER
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = RedfishAddendumLib| DXE_DRIVER
> > UEFI_DRIVER
> > +
> > +#
> > +# VALID_ARCHITECTURES = IA32 X64 EBC
> > +#
> > +
> > +[Sources]
> > + RedfishAddendumLib.c
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + MdeModulePkg/MdeModulePkg.dec
> > + RedfishPkg/RedfishPkg.dec
> > + RedfishClientPkg/RedfishClientPkg.dec
> > +
> > +[LibraryClasses]
> > + BaseLib
> > + DebugLib
> > + UefiBootServicesTableLib
> > + MemoryAllocationLib
> > + UefiLib
> > + JsonLib
> > +
> > +[Protocols]
> > + gEdkIIRedfishResourceAddendumProtocolGuid ## CONSUMED ##
> > +
> > diff --git a/RedfishClientPkg/Include/Library/RedfishAddendumLib.h
> > b/RedfishClientPkg/Include/Library/RedfishAddendumLib.h
> > new file mode 100644
> > index 00000000..d77347fd
> > --- /dev/null
> > +++ b/RedfishClientPkg/Include/Library/RedfishAddendumLib.h
> > @@ -0,0 +1,65 @@
> > +/** @file
> > + This file defines the Redfish addendum library interface.
> > +
> > + Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All
> > + rights
> > reserved.
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#ifndef REDFISH_ADDENDUM_LIB_H_
> > +#define REDFISH_ADDENDUM_LIB_H_
> > +
> > +#include <Uefi.h>
> > +#include <Library/JsonLib.h>
> > +#include <Protocol/EdkIIRedfishResourceAddendumProtocol.h>
> > +
> > +/**
> > + Provide redfish resource with addendum data in given schema.
> > +
> > + @param[in] Uri Uri of input resource.
> > + @param[in] Schema Redfish schema string.
> > + @param[in] Version Schema version string.
> > + @param[in] JsonText Input resource in JSON format string.
> > + @param[out] JsonWithAddendum The input resource with addendum
> > value attached.
> > +
> > + @retval EFI_SUCCESS Addendum data is attached.
> > + @retval EFI_NOT_FOUND No addendum protocol is found in system.
> > + @retval EFI_UNSUPPORTED No addendum data is required in given
> > schema.
> > + @retval Others Some error happened.
> > +
> > +**/
> > +EFI_STATUS
> > +RedfishGetAddendumData (
> > + IN EFI_STRING Uri,
> > + IN CHAR8 *Schema,
> > + IN CHAR8 *Version,
> > + IN CHAR8 *JsonText,
> > + OUT CHAR8 **JsonWithAddendum
> > + );
> > +
> > +/**
> > + Provide redfish OEM resource with given schema information.
> > +
> > + @param[in] Uri Uri of input resource.
> > + @param[in] Schema Redfish schema string.
> > + @param[in] Version Schema version string.
> > + @param[in] JsonText Input resource in JSON format string.
> > + @param[out] JsonWithOem The input resource with OEM value
> > attached.
> > +
> > + @retval EFI_SUCCESS OEM data is attached.
> > + @retval EFI_NOT_FOUND No addendum protocol is found in system.
> > + @retval EFI_UNSUPPORTED No OEM data is required in given schema.
> > + @retval Others Some error happened.
> > +
> > +**/
> > +EFI_STATUS
> > +RedfishGetOemData (
> > + IN EFI_STRING Uri,
> > + IN CHAR8 *Schema,
> > + IN CHAR8 *Version,
> > + IN CHAR8 *JsonText,
> > + OUT CHAR8 **JsonWithOem
> > + );
> > +
> > +#endif
> > diff --git
> > a/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c
> > b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c
> > new file mode 100644
> > index 00000000..6aa6e3cc
> > --- /dev/null
> > +++
> > b/RedfishClientPkg/Library/RedfishAddendumLib/RedfishAddendumLib.c
> > @@ -0,0 +1,262 @@
> > +/** @file
> > + Redfish addendum library helps Redfish application to get addendum
> > +data
> > and OEM
> > + data from platform driver.
> > +
> > + Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All
> > + rights
> > reserved.
> > +
> > + SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <Uefi.h>
> > +#include <RedfishBase.h>
> > +
> > +#include <Library/UefiLib.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/MemoryAllocationLib.h> #include
> > +<Library/RedfishAddendumLib.h>
> > +
> > +/**
> > +
> > + Convert Unicode string to ASCII string. It's call responsibility to
> > + release
> > returned buffer.
> > +
> > + @param[in] UnicodeStr Unicode string to convert.
> > +
> > + @retval CHAR8 * ASCII string returned.
> > + @retval NULL Errors occur.
> > +
> > +**/
> > +CHAR8 *
> > +UnicodeToAscii (
> > + IN EFI_STRING UnicodeStr
> > + )
> > +{
> > + CHAR8 *AsciiStr;
> > + UINTN AsciiStrSize;
> > + EFI_STATUS Status;
> > +
> > + if (IS_EMPTY_STRING (UnicodeStr)) {
> > + return NULL;
> > + }
> > +
> > + AsciiStrSize = StrLen (UnicodeStr) + 1;
> > + AsciiStr = AllocatePool (AsciiStrSize);
> > + if (AsciiStr == NULL) {
> > + return NULL;
> > + }
> > +
> > + Status = UnicodeStrToAsciiStrS (UnicodeStr, AsciiStr,
> > + AsciiStrSize); if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "UnicodeStrToAsciiStrS failed: %r\n", Status));
> > + FreePool (AsciiStr);
> > + return NULL;
> > + }
> > +
> > + return AsciiStr;
> > +}
> > +
> > +/**
> > + Provide redfish resource with addendum data in given schema.
> > +
> > + @param[in] Uri Uri of input resource.
> > + @param[in] Schema Redfish schema string.
> > + @param[in] Version Schema version string.
> > + @param[in] JsonText Input resource in JSON format string.
> > + @param[out] JsonWithAddendum The input resource with addendum
> > value attached.
> > + It is the caller's responsibility to free this buffer.
> > +
> > + @retval EFI_SUCCESS Addendum data is attached.
> > + @retval EFI_NOT_FOUND No addendum protocol is found in system.
> > + @retval EFI_UNSUPPORTED No addendum data is required in given
> > schema.
> > + @retval Others Some error happened.
> > +
> > +**/
> > +EFI_STATUS
> > +RedfishGetAddendumData (
> > + IN EFI_STRING Uri,
> > + IN CHAR8 *Schema,
> > + IN CHAR8 *Version,
> > + IN CHAR8 *JsonText,
> > + OUT CHAR8 **JsonWithAddendum
> > + )
> > +{
> > + REDFISH_RESOURCE_SCHEMA_INFO SchemaInfo;
> > + EDKII_JSON_VALUE JsonValue;
> > + EFI_STATUS Status;
> > + EFI_HANDLE *HandleBuffer;
> > + UINTN NumberOfHandles;
> > + UINTN Index;
> > + EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL *Protocol;
> > +
> > + if (IS_EMPTY_STRING (Uri) || IS_EMPTY_STRING (Schema) ||
> > IS_EMPTY_STRING (Version) || IS_EMPTY_STRING (JsonText) ||
> > (JsonWithAddendum == NULL)) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + *JsonWithAddendum = NULL;
> > + SchemaInfo.Uri = UnicodeToAscii (Uri);
> > + SchemaInfo.Schema = Schema;
> > + SchemaInfo.Version = Version;
> > + NumberOfHandles = 0;
> > + HandleBuffer = NULL;
> > +
> > + JsonValue = JsonLoadString (JsonText, 0, NULL); if ((JsonValue ==
> > + NULL) || !JsonValueIsObject (JsonValue)) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + Status = gBS->LocateHandleBuffer (
> > + ByProtocol,
> > + &gEdkIIRedfishResourceAddendumProtocolGuid,
> > + NULL,
> > + &NumberOfHandles,
> > + &HandleBuffer
> > + );
> > + if (EFI_ERROR (Status)) {
> > + goto ON_RELEASE;
> > + }
> > +
> > + for (Index = 0; Index < NumberOfHandles; Index++) {
> > + Status = gBS->HandleProtocol (
> > + HandleBuffer[Index],
> > + &gEdkIIRedfishResourceAddendumProtocolGuid,
> > + (VOID **)&Protocol
> > + );
> > + if (EFI_ERROR (Status)) {
> > + continue;
> > + }
> > +
> > + Status = Protocol->ProvisioningCallback (Protocol, &SchemaInfo,
> > JsonValue);
> > + if (!EFI_ERROR (Status)) {
> > + *JsonWithAddendum = JsonDumpString (JsonValue,
> > EDKII_JSON_COMPACT);
> > + break;
> > + }
> > + }
> > +
> > +ON_RELEASE:
> > +
> > + if (HandleBuffer != NULL) {
> > + FreePool (HandleBuffer);
> > + }
> > +
> > + if (JsonValue != NULL) {
> > + JsonValueFree (JsonValue);
> > + }
> > +
> > + if (SchemaInfo.Uri != NULL) {
> > + FreePool (SchemaInfo.Uri);
> > + }
> > +
> > + return Status;
> > +}
> > +
> > +/**
> > + Provide redfish OEM resource with given schema information.
> > +
> > + @param[in] Uri Uri of input resource.
> > + @param[in] Schema Redfish schema string.
> > + @param[in] Version Schema version string.
> > + @param[in] JsonText Input resource in JSON format string.
> > + @param[out] JsonWithOem The input resource with OEM value
> > attached.
> > +
> > + @retval EFI_SUCCESS OEM data is attached.
> > + @retval EFI_NOT_FOUND No addendum protocol is found in system.
> > + @retval EFI_UNSUPPORTED No OEM data is required in given schema.
> > + @retval Others Some error happened.
> > +
> > +**/
> > +EFI_STATUS
> > +RedfishGetOemData (
> > + IN EFI_STRING Uri,
> > + IN CHAR8 *Schema,
> > + IN CHAR8 *Version,
> > + IN CHAR8 *JsonText,
> > + OUT CHAR8 **JsonWithOem
> > + )
> > +{
> > + REDFISH_RESOURCE_SCHEMA_INFO SchemaInfo;
> > + EDKII_JSON_VALUE JsonValue;
> > + EDKII_JSON_VALUE JsonValueOem;
> > + EFI_STATUS Status;
> > + EFI_HANDLE *HandleBuffer;
> > + UINTN NumberOfHandles;
> > + UINTN Index;
> > + EDKII_REDFISH_RESOURCE_ADDENDUM_PROTOCOL *Protocol;
> > +
> > + if (IS_EMPTY_STRING (Uri) || IS_EMPTY_STRING (Schema) ||
> > IS_EMPTY_STRING (Version) || IS_EMPTY_STRING (JsonText) ||
> > (JsonWithOem == NULL)) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + *JsonWithOem = NULL;
> > + SchemaInfo.Uri = UnicodeToAscii (Uri);
> > + SchemaInfo.Schema = Schema;
> > + SchemaInfo.Version = Version;
> > + JsonValue = NULL;
> > + JsonValueOem = NULL;
> > + NumberOfHandles = 0;
> > + HandleBuffer = NULL;
> > +
> > + JsonValue = JsonLoadString (JsonText, 0, NULL); if ((JsonValue ==
> > + NULL) || !JsonValueIsObject (JsonValue)) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + JsonValueOem = JsonValueInitObject (); if ((JsonValueOem == NULL)
> > + || !JsonValueIsObject (JsonValueOem)) {
> > + Status = EFI_OUT_OF_RESOURCES;
> > + goto ON_RELEASE;
> > + }
> > +
> > + Status = gBS->LocateHandleBuffer (
> > + ByProtocol,
> > + &gEdkIIRedfishResourceAddendumProtocolGuid,
> > + NULL,
> > + &NumberOfHandles,
> > + &HandleBuffer
> > + );
> > + if (EFI_ERROR (Status)) {
> > + goto ON_RELEASE;
> > + }
> > +
> > + for (Index = 0; Index < NumberOfHandles; Index++) {
> > + Status = gBS->HandleProtocol (
> > + HandleBuffer[Index],
> > + &gEdkIIRedfishResourceAddendumProtocolGuid,
> > + (VOID **)&Protocol
> > + );
> > + if (EFI_ERROR (Status)) {
> > + continue;
> > + }
> > +
> > + Status = Protocol->OemCallback (Protocol, &SchemaInfo, JsonValueOem);
> > + if (!EFI_ERROR (Status)) {
> > + Status = JsonObjectSetValue (JsonValue, "Oem", JsonValueOem);
> > + if (!EFI_ERROR (Status)) {
> > + *JsonWithOem = JsonDumpString (JsonValue, EDKII_JSON_COMPACT);
> > + }
> > +
> > + break;
> > + }
> > + }
> > +
> > +ON_RELEASE:
> > +
> > + if (HandleBuffer != NULL) {
> > + FreePool (HandleBuffer);
> > + }
> > +
> > + if (JsonValue != NULL) {
> > + JsonValueFree (JsonValue);
> > + }
> > +
> > + if (JsonValueOem != NULL) {
> > + JsonValueFree (JsonValueOem);
> > + }
> > +
> > + if (SchemaInfo.Uri != NULL) {
> > + FreePool (SchemaInfo.Uri);
> > + }
> > +
> > + return Status;
> > +}
> > --
> > 2.17.1
prev parent reply other threads:[~2023-05-15 7:39 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-12 7:23 [edk2-redfish-client][PATCH 2/3] RedfishClientPkg: Add Redfish Resource Addendum Library Nickle Wang
2023-05-15 3:19 ` Chang, Abner
2023-05-15 7:39 ` Nickle Wang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MW4PR12MB7031CDC04C1614A7B331E5CFD9789@MW4PR12MB7031.namprd12.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox