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 D0CCC740035 for ; Mon, 18 Dec 2023 00:33:07 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=DKASqbGDF7QbKcSoI8Cyr3iQYPdRO+XlH52fyPjA6VQ=; 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=1702859586; v=1; b=pdgqKN/lLa2LnGNbNYB2MGLIdFwm/cA517ap6cyxxWnDsrZ5Z/o4AoHT3itadInd+W3xIPJq +V5FPawu0nF/0BcmFxh5XKNCbg2vV6d9APYhIsPh6fH6kwENX16yr1684mFoptNfgSbuOTx+8Ko 5ITJShrwD2ZSL5ob/hg8qKuk= X-Received: by 127.0.0.2 with SMTP id cPmnYY7687511xcTrG8Vc8Vm; Sun, 17 Dec 2023 16:33:06 -0800 X-Received: from mail-lj1-f176.google.com (mail-lj1-f176.google.com [209.85.208.176]) by mx.groups.io with SMTP id smtpd.web11.30971.1702859585478481209 for ; Sun, 17 Dec 2023 16:33:05 -0800 X-Received: by mail-lj1-f176.google.com with SMTP id 38308e7fff4ca-2cc4029dc6eso28844671fa.1 for ; Sun, 17 Dec 2023 16:33:05 -0800 (PST) X-Gm-Message-State: xZljEzSeP6qcwQ2K6it4ZdP9x7686176AA= X-Google-Smtp-Source: AGHT+IHxK+jWPGpLsQWRfECRRXQFdJi5dYwzW2njm3oju6ZnDePNIH5QCzcI13YRIf/m76E7fpxZMA== X-Received: by 2002:a2e:9403:0:b0:2c9:eff2:b4ea with SMTP id i3-20020a2e9403000000b002c9eff2b4eamr7259215ljh.75.1702859583290; Sun, 17 Dec 2023 16:33:03 -0800 (PST) X-Received: from smtpclient.apple ([79.164.221.98]) by smtp.gmail.com with ESMTPSA id f10-20020a05651c03ca00b002cc565cc5e3sm694575ljp.68.2023.12.17.16.33.02 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 17 Dec 2023 16:33:02 -0800 (PST) From: "Mike Maslenkin" Message-Id: 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:33:01 +0300 In-Reply-To: Cc: "devel@edk2.groups.io" , "abner.chang@amd.com" , "igork@ami.com" To: Nickle Wang References: <20231215000400.5311-1-mike.maslenkin@gmail.com> <20231215000400.5311-5-mike.maslenkin@gmail.com> 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=_DAFC8A0F-84A4-4181-9385-9F7ABA0C257C" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="pdgqKN/l"; 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=_DAFC8A0F-84A4-4181-9385-9F7ABA0C257C Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii Hi Nickle, > 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 ne= w resource will belong." >=20 > If this is not a collection resource, we cannot use POST method. And /red= fish/v1/Systems/SYS_ID/Bios is not a collection resource. The allowed metho= d 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 th= at the function header below is confusing and did not express above rule cl= early. >=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_EXCHANG= E. > @param[in] HttpPostMode TRUE if resource does not exist, HTTP P= OST method is used. > FALSE if the resource exist but some of= 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 targ= et 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 Seems like more comments need to be changed.... The idea behind this patch is the basis of the current implementation of Re= dfishClientPkg/Features/Bios/v1_0_9. The EdkIIRedfishResourceConfigProvisioning function calls the EDKII_REDFISH= _RESOURCE_CONFIG_PROTOCOL::Provisioning function, this is RedfishResourceProvisioningResource() for this driver[1], then it c= alls RedfishProvisioningResourceCommon() where the logic of this flag is completely changed [2]. So, for the current implementation of this flag, it is not a choice between= 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 Pro= visioningBiosExistResource() function calls ProvisioningBiosProperties() with ProvisionMode =3D=3D TRUE. This Provision= Mode =3D=3D TRUE forces PropertyChanged set into TRUE, so finally ProvisioningBiosProperties() returns success even for elements t= hat do not exist. Here I mean elements of /redfish/v1/Systems/{SystemID}/Bios/Attributes. I'm= in situation when Attributes exists, but it is empty. Currently the PUT method is not used anywhere in RedfishClientPkg/Features/= Bios and "PUT back to instance" actually performs the PATCH. I will drop this patch from the current PR until it becomes clear how this = can be improved. 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 met= hod >> 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 be >> used. Fourth parameter of EdkIIRedfishResourceConfigProvisioning is BOOL= EAN >> 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 pr= operty. >>=20 >> // >>=20 >> DEBUG ((REDFISH_DEBUG_TRACE, "%a provision for %s\n", __func__, Uri)= ); >>=20 >> - Status =3D EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri= , Private- >>> InformationExchange, FALSE); >>=20 >> + Status =3D EdkIIRedfishResourceConfigProvisioning (&SchemaInfo, Uri= , >> + 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 -=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 (#112610): https://edk2.groups.io/g/devel/message/112610 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=_DAFC8A0F-84A4-4181-9385-9F7ABA0C257C Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii

Hi Nickle,

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

Hi Mike,

Per Redfish= specification 7.9 POST(create)

"The POST requ= est is submitted to the resource collection to which the new resource will = belong."

If this is not a collection resource,= we cannot use POST method. And /redfish/v1/Systems/SYS_ID/Bios is not a co= llection resource. The allowed method returned from BMC for BIOS resource i= s usually "GET" and "PUT".

So, I think that th= e fourth parameter is still FALSE here. But I admit that the function heade= r below is confusing and did not express above rule clearly.
=
/**
 Provisioning redfish resource by gi= ven URI.

 @param[in]   Schema =             &nb= sp;Redfish schema information.
 @param[in]   = Uri             = ;    Target URI to create resource.
&nbs= p;@param[in]   InformationExchange Pointer to RESOURCE_INFORMATIO= N_EXCHANGE.
 @param[in]   HttpPostMode  =       TRUE if resource does not exist, HTTP P= OST method is used.
      &nbs= p;            &= nbsp;           &nbs= p;  FALSE if the resource exist but some of properties are missin= g,
         &nb= sp;            =             HTT= P PUT method is used.

 @retval EFI_SUCCE= SS             =  Value is returned successfully.
 @retval Others &= nbsp;           &nbs= p;     Some error happened.

**/

Below is my suggestion.

 @param[in]   HttpPostMode   &n= bsp;    TRUE if target resource is a member of collecti= on resource, HTTP POST method is used.
   &nb= sp;            =             &nb= sp;            =             &nb= sp;      FALSE if target resource is non-coll= ection resource, HTTP PUT method is used.

Do y= ou think this helps to explain the use-case of fourth parameter more clearl= y?

Thanks,
Nickle
=


Seems like mor= e 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 EdkIIRedfi= shResourceConfigProvisioning function calls the EDKII_REDFISH_RESOURCE_CONF= IG_PROTOCOL::Provisioning function,
this is RedfishResourceProvisioningResource(= ) for this driver[1], then it calls RedfishProvisioningResourceCommon() whe= re the logic
of this flag is completely changed [2].
So, for the current implementati= on of this flag, it is not a choice between POST and PUT, but between POST = and 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.

Regards,
Mike.

-----Original Message-----
From: Mike Maslenkin <mike.maslenkin@gmail.com>
Sent: Friday, December 15, 2023 8:04 AM
T= o: devel@edk2.groups.io<= /a>
Cc: abn= er.chang@amd.com; Nickle Wang <nicklew@nvidia.com>;
igork@ami.com; Mike Maslenkin <mike.maslenkin@gmail.com>Subject: [edk2-redfish-client][PATCH 4/4] RedfishClientPkg: us= e POST method
while provisioning new property.
=
External email: Use caution opening links or attachments


If EdkIIRedfishResourceConfigCheck= fails according to the logic and
comment: new resources shou= ld be provisioned, so the POST method must be
used. Fourth pa= rameter of EdkIIRedfishResourceConfigProvisioning is BOOLEAN
= HttpPostMode, so we pass TRUE here.

Cc: Abner = Chang <abner.chang@amd= .com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Nickle Wang &l= t;nicklew@nvidia.com&g= t;
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
---
RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosComm= on.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-= )

diff --git a/RedfishClientPkg/Features/Bios/= v1_0_9/Common/BiosCommon.c
b/RedfishClientPkg/Features/Bios/v= 1_0_9/Common/BiosCommon.c
index a26a1083cd74..4fd4845f3420 10= 0644
--- a/RedfishClientPkg/Features/Bios/v1_0_9/Common/BiosC= ommon.c
+++ b/RedfishClientPkg/Features/Bios/v1_0_9/Common/Bi= osCommon.c
@@ -818,9 +818,9 @@ HandleResource (
    // The target property does not exist, do the pro= vision to create property.

   =  //

    DEBUG ((REDF= ISH_DEBUG_TRACE, "%a provision for %s\n", __func__, Uri));
-    Status =3D EdkIIRedfishResourceConfigProvi= sioning (&SchemaInfo, Uri, Private-
InformationExchange, FALSE);
+    Status =3D EdkIIRedfishResourceConfigProvis= ioning (&SchemaInfo, Uri,
+ Private->InformationExchan= ge, TRUE);

    if (EFI_ER= ROR (Status)) {

-     &nbs= p;DEBUG ((DEBUG_ERROR, "%a, failed to provision with GET mode: %r\n",
__func__, Status));

+   &nb= sp;  DEBUG ((DEBUG_ERROR, "%a, failed to provision with POST mode= :
+ %r\n", __func__, Status));

=     }



    return Status;

--
2.32.0 (Apple Git-132)

<= div>

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

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

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

_._,_._,_
--Apple-Mail=_DAFC8A0F-84A4-4181-9385-9F7ABA0C257C--