From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 840C19406D5 for ; Mon, 18 Dec 2023 00:58:16 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=oG7v8a8WZ2ARRsGZwwkxEarjy7/qbz+K4qY2GjcrEdA=; c=relaxed/simple; d=groups.io; h=From:Message-Id:Mime-Version:Subject:Date:In-Reply-To:Cc:To:References:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1702861095; v=1; b=OhA5lmebtMbPcCg+QtHYZXlv2WYom2KHdey2CpZxduUleVlBLjY+wnzqk7ETNtsCsrFSDIUk 24tRNfH8jtN+7VP8whv3YXCG+6Lw8dWVvY3jQqqrTzxOOH7ar9YQrxgBvwr1ysdogf1EIFC2LUN T7WYvMD0vMjcSUSIAuLJVbW4= X-Received: by 127.0.0.2 with SMTP id qAOEYY7687511xrvB4bM1chs; Sun, 17 Dec 2023 16:58:15 -0800 X-Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) by mx.groups.io with SMTP id smtpd.web10.31361.1702861093963656484 for ; Sun, 17 Dec 2023 16:58:14 -0800 X-Received: by mail-lf1-f49.google.com with SMTP id 2adb3069b0e04-50e2ce4fb22so1315906e87.1 for ; Sun, 17 Dec 2023 16:58:13 -0800 (PST) X-Gm-Message-State: b8IvupGLoAErXVbH1bCm8Z6sx7686176AA= X-Google-Smtp-Source: AGHT+IHJECEa5rW6sxwNMM0cXn9Lt7VllhkQfz4jpPgrDSBbMRVcMUT6fN/7t19JLG415S47IMtwtA== X-Received: by 2002:a05:6512:48b:b0:50e:3089:a6b5 with SMTP id v11-20020a056512048b00b0050e3089a6b5mr1038366lfq.76.1702861091685; Sun, 17 Dec 2023 16:58:11 -0800 (PST) X-Received: from smtpclient.apple ([79.164.221.98]) by smtp.gmail.com with ESMTPSA id b20-20020a056512071400b0050bc297e001sm2752814lfs.280.2023.12.17.16.58.11 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 17 Dec 2023 16:58:11 -0800 (PST) From: "Mike Maslenkin" Message-Id: <034F184D-20DC-4FFC-A40B-2426CF0085CD@gmail.com> Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.100.31\)) Subject: Re: [edk2-devel] [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST method while provisioning new property. Date: Mon, 18 Dec 2023 03:57:45 +0300 In-Reply-To: <17A1C5C572E0EF8A.24236@groups.io> Cc: Nickle Wang , "abner.chang@amd.com" , "igork@ami.com" To: devel@edk2.groups.io, mike.maslenkin@gmail.com References: <20231215000400.5311-1-mike.maslenkin@gmail.com> <20231215000400.5311-5-mike.maslenkin@gmail.com> <17A1C5C572E0EF8A.24236@groups.io> Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mike.maslenkin@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="Apple-Mail=_0CF63E95-1E07-48F4-AEF1-8B1C442710EE" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=OhA5lmeb; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --Apple-Mail=_0CF63E95-1E07-48F4-AEF1-8B1C442710EE Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > On 18. 12. 2023., at 03:33, Mike Maslenkin via groups.io wrote: >=20 >=20 > Hi Nickle, >=20 >> On 15. 12. 2023., at 04:53, Nickle Wang > wrote: >>=20 >> Hi Mike, >>=20 >> Per Redfish specification 7.9 POST(create) >>=20 >> "The POST request is submitted to the resource collection to which the n= ew resource will belong." >>=20 >> If this is not a collection resource, we cannot use POST method. And /re= dfish/v1/Systems/SYS_ID/Bios is not a collection resource. The allowed meth= od returned from BMC for BIOS resource is usually "GET" and "PUT". >>=20 >> So, I think that the fourth parameter is still FALSE here. But I admit t= hat the function header below is confusing and did not express above rule c= learly. >>=20 >> /** >> Provisioning redfish resource by given URI. >>=20 >> @param[in] Schema Redfish schema information. >> @param[in] Uri Target URI to create resource. >> @param[in] InformationExchange Pointer to RESOURCE_INFORMATION_EXCHAN= GE. >> @param[in] HttpPostMode TRUE if resource does not exist, HTTP = POST method is used. >> FALSE if the resource exist but some o= f properties are missing, >> HTTP PUT method is used. >>=20 >> @retval EFI_SUCCESS Value is returned successfully. >> @retval Others Some error happened. >>=20 >> **/ >>=20 >> Below is my suggestion. >>=20 >> @param[in] HttpPostMode TRUE if target resource is a member of= collection resource, HTTP POST method is used. >> FALSE if tar= get resource is non-collection resource, HTTP PUT method is used. >>=20 >> Do you think this helps to explain the use-case of fourth parameter more= clearly? >>=20 >> Thanks, >> Nickle >=20 >=20 > Seems like more comments need to be changed.... > The idea behind this patch is the basis of the current implementation of = RedfishClientPkg/Features/Bios/v1_0_9. > The EdkIIRedfishResourceConfigProvisioning function calls the EDKII_REDFI= SH_RESOURCE_CONFIG_PROTOCOL::Provisioning function, > this is RedfishResourceProvisioningResource() for this driver[1], then it= calls RedfishProvisioningResourceCommon() where the logic > of this flag is completely changed [2]. > So, for the current implementation of this flag, it is not a choice betwe= en POST and PUT, but between POST and PATCH (see [1]). > May be flag should not be inverted here [3]?=20 > The value of FALSE here (you expect PUT to be used) means the following P= rovisioningBiosExistResource() function calls > ProvisioningBiosProperties() with ProvisionMode =3D=3D TRUE. This Provisi= onMode =3D=3D TRUE forces PropertyChanged set into TRUE, > so finally ProvisioningBiosProperties() returns success even for elements= that do not exist. >=20 > Here I mean elements of /redfish/v1/Systems/{SystemID}/Bios/Attributes. I= 'm in situation when Attributes exists, > but it is empty. >=20 > Currently the PUT method is not used anywhere in RedfishClientPkg/Feature= s/Bios and "PUT back to instance" actually performs the PATCH. >=20 > I will drop this patch from the current PR until it becomes clear how thi= s can be improved. Forgot to add the links: [1] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClien= tPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c#L50 [2] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClien= tPkg/Features/Bios/v1_0_9/Common/BiosCommon.c#L518 [3] https://github.com/tianocore/edk2-redfish-client/blob/main/RedfishClien= tPkg/Features/Bios/v1_0_9/Dxe/BiosDxe.c#L68 >=20 > Regards, > Mike. >>=20 >>> -----Original Message----- >>> From: Mike Maslenkin > >>> Sent: Friday, December 15, 2023 8:04 AM >>> To: devel@edk2.groups.io >>> Cc: abner.chang@amd.com ; Nickle Wang >; >>> igork@ami.com ; Mike Maslenkin > >>> Subject: [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: use POST me= thod >>> while provisioning new property. >>>=20 >>> External email: Use caution opening links or attachments >>>=20 >>>=20 >>> If EdkIIRedfishResourceConfigCheck fails according to the logic and >>> comment: new resources should be provisioned, so the POST method must b= e >>> used. Fourth parameter of EdkIIRedfishResourceConfigProvisioning is BOO= LEAN >>> HttpPostMode, so we pass TRUE here. >>>=20 >>> Cc: Abner Chang > >>> Cc: Igor Kulchytskyy > >>> Cc: Nickle Wang > >>> Signed-off-by: Mike Maslenkin > >>> --- >>> RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>=20 >>> diff --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c >>> b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c >>> index a26a1083cd74..4fd4845f3420 100644 >>> --- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c >>> +++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c >>> @@ -818,9 +818,9 @@ HandleResource ( >>> // The target property does not exist, do the provision to create p= roperty. >>>=20 >>> // >>>=20 >>> DEBUG ((REDFISH_DEBUG_TRACE, "%a provision for %s\n", __func__, Uri= )); >>>=20 >>> - Status =3D EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Ur= i, Private- >>>> InformationExchange, FALSE); >>>=20 >>> + Status =3D EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Ur= i, >>> + Private->InformationExchange, TRUE); >>>=20 >>> if (EFI_ERROR (Status)) { >>>=20 >>> - DEBUG ((DEBUG_ERROR, "%a, failed to provision with GET mode: %r\= n", >>> __func__, Status)); >>>=20 >>> + DEBUG ((DEBUG_ERROR, "%a, failed to provision with POST mode: >>> + %r\n", __func__, Status)); >>>=20 >>> } >>>=20 >>>=20 >>>=20 >>> return Status; >>>=20 >>> -- >>> 2.32.0 (Apple Git-132) >=20 >>=20 >=20 >=20 >=20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112611): https://edk2.groups.io/g/devel/message/112611 Mute This Topic: https://groups.io/mt/103181641/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --Apple-Mail=_0CF63E95-1E07-48F4-AEF1-8B1C442710EE Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii
On 18. 12.= 2023., at 03:33, Mike Maslenkin via groups.io <mike.maslenkin=3Dgmail.com@groups.io> wrote:


Hi Nickle,=

On 15. 12. 2023., at 04:53, Nickle Wang <nicklew@nvidia.com> wrote:=

Hi Mike,

Per Redfish specification 7.9 POST= (create)

"The POST request is submitted to the= resource collection to which the new resource will belong."
=
If this is not a collection resource, we cannot use POST met= hod. And /redfish/v1/Systems/SYS_ID/Bios is not a collection resource. The = allowed method returned from BMC for BIOS resource is usually "GET" and "PU= T".

So, I think that the fourth parameter is s= till FALSE here. But I admit that the function header below is confusing an= d did not express above rule clearly.

/**
 Provisioning redfish resource by given URI.
=
 @param[in]   Schema     = ;         Redfish schema infor= mation.
 @param[in]   Uri    &= nbsp;           &nbs= p;Target URI to create resource.
 @param[in]  &nbs= p;InformationExchange Pointer to RESOURCE_INFORMATION_EXCHANGE.
 @param[in]   HttpPostMode      = ;  TRUE if resource does not exist, HTTP POST method is used.
          &nb= sp;            =            FALSE if = the resource exist but some of properties are missing,
 = ;            &n= bsp;            = ;        HTTP PUT method is used.
 @retval EFI_SUCCESS    &n= bsp;         Value is returned= successfully.
 @retval Others     =             &nb= sp; Some error happened.

**/

Below is my suggestion.

&nbs= p;@param[in]   HttpPostMode       &= nbsp;TRUE if target resource is a member of collection resource, HTTP POST = method is used.
       &n= bsp;            = ;            &n= bsp;            = ;            &n= bsp;  FALSE if target resource is non-collection resource, HTTP P= UT method is used.

Do you think this helps to = explain the use-case of fourth parameter more clearly?

Thanks,
Nickle


Seems like more= comments need to be changed....
The idea behind this patch is the basis of the = current implementation of RedfishClientPkg/Features/Bios/v1_0_9.
The EdkIIRedfis= hResourceConfigProvisioning function calls the EDKII_REDFISH_RESOURCE_CONFI= G_PROTOCOL::Provisioning function,
this is RedfishResourceProvisioningResource()= for this driver[1], then it calls RedfishProvisioningResourceCommon() wher= e the logic
of this flag is completely changed [2].
So, for the current implementatio= n of this flag, it is not a choice between POST and PUT, but between POST a= nd PATCH (see [1]).
May be flag should not be inverted here [= 3]? 
The value of FALSE here (you expect PUT to be used) means the followi= ng ProvisioningBiosExistResource() function calls
ProvisioningBiosPropertie= s() with ProvisionMode =3D=3D TRUE. This ProvisionMode =3D=3D TRUE forces P= ropertyChanged set into TRUE,
so finally ProvisioningBiosProperties() returns su= ccess even for elements that do not exist.

Here I mean elements of /r= edfish/v1/Systems/{SystemID}/Bios/Attributes. I'm in situation when Attribu= tes exists,
but it is empty.

Currently the PUT method is not used anywhere= in RedfishClientPkg/Features/Bios and "PUT back to instance" actually perf= orms the PATCH.

I= will drop this patch from the current PR until it becomes clear how t= his can be improved.

Forgot to add the links:


-----Original Message-----
From: Mike Maslenkin <mike.maslenkin@gmail.com>
Sent: Friday, = December 15, 2023 8:04 AM
To: devel@edk2.groups.io
Cc: abner.chang@amd.com; Nickle Wang = <nicklew@nvidia.com= >;
igork@ami.c= om; Mike Maslenkin <mike.maslenkin@gmail.com>
Subject: [edk2-redfis= h-client][PATCH 4/4] RedfishClientPkg: use POST method
while = provisioning new property.

External email: Use= caution opening links or attachments


If EdkIIRedfishResourceConfigCheck fails according to the logic and<= br class=3D"">comment: new resources should be provisioned, so the POST met= hod must be
used. Fourth parameter of EdkIIRedfishResourceCon= figProvisioning is BOOLEAN
HttpPostMode, so we pass TRUE here= .

Cc: Abner Chang <abner.chang@amd.com>
Cc: Igo= r Kulchytskyy <igork@ami.com= >
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Mi= ke Maslenkin <mik= e.maslenkin@gmail.com>
---
RedfishClien= tPkg/Features/Bios/v1_0_9/Common/BiosCommon.c | 4 ++--
1 fil= e changed, 2 insertions(+), 2 deletions(-)

dif= f --git a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
b/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
index a26a1083cd74..4fd4845f3420 100644
--- a/RedfishCl= ientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
+++ b/Redfis= hClientPkg/Features/Bios/v1_0_9/Common/BiosCommon.c
@@ -818,9= +818,9 @@ HandleResource (
    // The t= arget property does not exist, do the provision to create property.

    //

    DEBUG ((REDFISH_DEBUG_TRACE, "%a provision f= or %s\n", __func__, Uri));

-    = ;Status =3D EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri, P= rivate-
InformationExcha= nge, FALSE);

+    = Status =3D EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri,+ Private->InformationExchange, TRUE);

    if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a, fail= ed to provision with GET mode: %r\n",
__func__, Status));

+      DEBUG ((DEBUG_ERR= OR, "%a, failed to provision with POST mode:
+ %r\n", __func_= _, Status));

    }



   &nb= sp;return Status;

--
2.32.0 (App= le Git-132)



_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#112611) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--Apple-Mail=_0CF63E95-1E07-48F4-AEF1-8B1C442710EE--