Re: [EFI Shell PATCH v2 1/2] MdePkg/Include: EFI Redfish Discover protocol

["Abner Chang" <abner.chang@hpe.com>]

Liming, feedbacks in below.

> -----Original Message-----
> From: gaoliming [mailto:gaoliming@byosoft.com.cn]
> Sent: Monday, November 9, 2020 9:40 AM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> devel@edk2.groups.io
> Cc: 'Michael D Kinney' <michael.d.kinney@intel.com>; 'Zhiguang Liu'
> <zhiguang.liu@intel.com>; Wang, Nickle (HPS SW) <nickle.wang@hpe.com>;
> O'Hanley, Peter (EXL) <peter.ohanley@hpe.com>
> Subject: 回复: [EFI Shell PATCH v2 1/2] MdePkg/Include: EFI Redfish Discover
> protocol
> 
> Abner:
>  I further check UEFI Spec 2.8 A and review the code change. I find some
> structure definitions are different. Can you make sure the one in MdePkg to
> align to UEFI spec?
> 
>  Now, I find these below three structure definition don't follow UEFI spec.
> Please help check them.
>  EFI_REDFISH_DISCOVERED_INFORMATION,
> EFI_REDFISH_DISCOVERED_INSTANCE and
> EFI_REDFISH_DISCOVERED_TOKEN

I also found some inconsistencies between UEFI spec and the implementation. Some definitions are changed when we implement the code.
ECR is submitted to Mantis,
https://mantis.uefi.org/mantis/view.php?id=2172
We can just halt this patch until the ECR is approved.

> 
>  Besides, the driver should include the header file that defines Guid if the
> driver uses those Guid. So, those two guids are not required to be listed here.
> 
> extern EFI_GUID gEfiRestExProtocolGuid;
> extern EFI_GUID gEfiRestExServiceBindingProtocolGuid;
Ok, will remove these guids.

> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: Abner Chang <abner.chang@hpe.com>
> > 发送时间: 2020年11月6日 15:48
> > 收件人: devel@edk2.groups.io
> > 抄送: Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>;
> > Nickle Wang <nickle.wang@hpe.com>; Peter O'Hanley
> > <peter.ohanley@hpe.com>
> > 主题: [EFI Shell PATCH v2 1/2] MdePkg/Include: EFI Redfish Discover
> protocol
> >
> > Move GUID definition of EFI Redfish Discover protocol to under MdePkg.
> > With this we don't have dependency of RedfishPkg in ShellPkg.
> >
> > Signed-off-by: Abner Chang <abner.chang@hpe.com>
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Nickle Wang <nickle.wang@hpe.com>
> > Cc: Peter O'Hanley <peter.ohanley@hpe.com>
> > ---
> >  .../Include/Protocol/RedfishDiscover.h        | 42 ++++++++-----------
> >  MdePkg/MdePkg.dec                             |  3 ++
> >  RedfishPkg/RedfishPkg.dec                     |  4 --
> >  3 files changed, 21 insertions(+), 28 deletions(-)  rename
> > {RedfishPkg => MdePkg}/Include/Protocol/RedfishDiscover.h (87%)
> >
> > diff --git a/RedfishPkg/Include/Protocol/RedfishDiscover.h
> > b/MdePkg/Include/Protocol/RedfishDiscover.h
> > similarity index 87%
> > rename from RedfishPkg/Include/Protocol/RedfishDiscover.h
> > rename to MdePkg/Include/Protocol/RedfishDiscover.h
> > index 4c91605c4e..8fe23470e1 100644
> > --- a/RedfishPkg/Include/Protocol/RedfishDiscover.h
> > +++ b/MdePkg/Include/Protocol/RedfishDiscover.h
> > @@ -5,16 +5,13 @@
> >
> >    SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > +  @par Revision Reference:
> > +  This Protocol is introduced in UEFI Specification 2.8
> >  **/
> >
> >  #ifndef EFI_REDFISH_DISCOVER_PROTOCOL_H_  #define
> > EFI_REDFISH_DISCOVER_PROTOCOL_H_
> >
> > -#include <IndustryStandard/Http11.h>
> > -#include <Protocol/Http.h>
> > -#include <Protocol/RestEx.h>
> > -#include <Uefi.h>
> > -
> >  //
> >  // GUID definitions
> >  //
> > @@ -40,12 +37,9 @@ typedef UINT32 EFI_REDFISH_DISCOVER_FLAG;
> >                                                         ///< 3 to 15.
> > The corresponding duration is 8 to 2^15 seconds.
> >                                                         ///< Duration
> > is only valid when EFI_REDFISH_DISCOVER_KEEP_ALIVE
> >                                                         ///< is set to
> > 1.
> > -#define EFI_REDFISH_DISCOVER_DURATION_BIT_POS 8
> > -
> >  typedef struct _EFI_REDFISH_DISCOVER_PROTOCOL
> > EFI_REDFISH_DISCOVER_PROTOCOL; -typedef struct
> > _EFI_REDFISH_DISCOVERED_INFORMATION
> > EFI_REDFISH_DISCOVERED_INFORMATION;
> >
> > -typedef struct _EFI_REDFISH_DISCOVERED_INFORMATION {
> > +typedef struct {
> >    EFI_HANDLE RedfishRestExHandle;           ///< REST EX EFI handle
> > associated with this Redfish service.
> >    BOOLEAN         IsUdp6;                   ///< Indicates it's IP
> > versino 6.
> >    EFI_IP_ADDRESS  RedfishHostIpAddress;     ///< IP address of Redfish
> > service.
> > @@ -57,7 +51,7 @@ typedef struct
> > _EFI_REDFISH_DISCOVERED_INFORMATION {
> >    CHAR16 *Product;                          ///< Redfish service
> > product name.
> >    CHAR16 *ProductVer;                       ///< Redfish service
> > product version.
> >    BOOLEAN UseHttps;                         ///< Using HTTPS.
> > -};
> > +} EFI_REDFISH_DISCOVERED_INFORMATION;
> >
> >  typedef struct {
> >    EFI_STATUS Status;                                ///< Status of
> > Redfish service discovery.
> > @@ -75,7 +69,7 @@ typedef struct {
> >      EFI_IP_ADDRESS        SubnetId;               ///< Subnet ID.
> >      UINT8                 SubnetPrefixLength;     ///< Subnet
> > prefix-length for IPv4 and IPv6.
> >      UINT16                VlanId;                 ///< VLAN ID.
> > -} EFI_REDFISH_DISCOVER_NETWORK_INSTANCE;
> > +} EFI_REDFISH_DISCOVER_NETWORK_INTERFACE;
> >
> >  typedef struct {
> >    UINT32    Signature;            ///< Token signature.
> > @@ -112,10 +106,10 @@ typedef struct {  typedef  EFI_STATUS  (EFIAPI
> > *EFI_REDFISH_DISCOVER_NETWORK_LIST)(
> > -  IN EFI_REDFISH_DISCOVER_PROTOCOL   *This,
> > -  IN EFI_HANDLE                      ImageHandle,
> > -  OUT UINTN
> > *NumberOfNetworkInterfaces,
> > -  OUT EFI_REDFISH_DISCOVER_NETWORK_INSTANCE
> > **NetworkInterfaces
> > +  IN EFI_REDFISH_DISCOVER_PROTOCOL           *This,
> > +  IN EFI_HANDLE                              ImageHandle,
> > +  OUT UINTN
> > *NumberOfNetworkInterfaces,
> > +  OUT EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > **NetworkInterfaces
> >  );
> >
> >  /**
> > @@ -147,7 +141,7 @@ EFI_STATUS
> >  (EFIAPI *EFI_REDFISH_DISCOVER_ACQUIRE_SERVICE)(
> >    IN EFI_REDFISH_DISCOVER_PROTOCOL          *This,
> >    IN EFI_HANDLE                             ImageHandle,
> > -  IN EFI_REDFISH_DISCOVER_NETWORK_INSTANCE
> > *TargetNetworkInterface,
> > +  IN EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *TargetNetworkInterface OPTIONAL,
> >    IN EFI_REDFISH_DISCOVER_FLAG              Flags,
> >    IN EFI_REDFISH_DISCOVERED_TOKEN           *Token
> >  );
> > @@ -155,8 +149,8 @@ EFI_STATUS
> >  /**
> >    This function aborts Redfish service discovery on the given network
> > interface.
> >
> > -  @param[in]    This          EFI_REDFISH_DISCOVER_PROTOCOL
> > instance.
> > -  @param[in]    TargetNetworkInterface     Target NIC to do the
> > discovery.
> > +  @param[in]    This
> > EFI_REDFISH_DISCOVER_PROTOCOL instance.
> > +  @param[in]    TargetNetworkInterface  Target NIC to do the
> > discovery.
> >
> >    @retval EFI_SUCCESS             REST EX instance of discovered
> > Redfish services are returned.
> >    @retval Others                  Fail to abort Redfish service
> > discovery.
> > @@ -165,15 +159,15 @@ EFI_STATUS
> >  typedef
> >  EFI_STATUS
> >  (EFIAPI *EFI_REDFISH_DISCOVER_ABORT_ACQUIRE)(
> > -  IN EFI_REDFISH_DISCOVER_PROTOCOL      *This,
> > -  IN EFI_REDFISH_DISCOVER_NETWORK_INSTANCE
> > *TargetNetworkInterface OPTIONAL
> > +  IN EFI_REDFISH_DISCOVER_PROTOCOL          *This,
> > +  IN EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> > *TargetNetworkInterface OPTIONAL
> >  );
> >
> >  /**
> >    This function releases Redfish services found by
> RedfishServiceAcquire().
> >
> >    @param[in]    This         EFI_REDFISH_DISCOVER_PROTOCOL
> > instance.
> > -  @param[in]    InstanceList The Redfish service to release.
> > +  @param[in]    List         The Redfish service to release.
> >
> >    @retval EFI_SUCCESS        REST EX instances of discovered Redfish
> > are released.
> >    @retval Others             Fail to remove the entry
> > @@ -183,15 +177,15 @@ typedef
> >  EFI_STATUS
> >  (EFIAPI *EFI_REDFISH_DISCOVER_RELEASE_SERVICE)(
> >    IN EFI_REDFISH_DISCOVER_PROTOCOL   *This,
> > -  IN EFI_REDFISH_DISCOVERED_LIST *InstanceList
> > +  IN EFI_REDFISH_DISCOVERED_LIST     *List
> >  );
> >
> > -typedef struct _EFI_REDFISH_DISCOVER_PROTOCOL {
> > +struct _EFI_REDFISH_DISCOVER_PROTOCOL {
> >    EFI_REDFISH_DISCOVER_NETWORK_LIST    GetNetworkInterfaceList;
> >    EFI_REDFISH_DISCOVER_ACQUIRE_SERVICE AcquireRedfishService;
> >    EFI_REDFISH_DISCOVER_ABORT_ACQUIRE
> > AbortAcquireRedfishService;
> >    EFI_REDFISH_DISCOVER_RELEASE_SERVICE ReleaseRedfishService; -}
> > EFI_REDFISH_DISCOVER_PROTOCOL;
> > +};
> >
> >  extern EFI_GUID gEfiRestExProtocolGuid;  extern EFI_GUID
> > gEfiRestExServiceBindingProtocolGuid;
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
> > 645f61b80e..839bcfe063 100644
> > --- a/MdePkg/MdePkg.dec
> > +++ b/MdePkg/MdePkg.dec
> > @@ -1858,6 +1858,9 @@
> >    ## Include/Protocol/RestJsonStructure.h
> >    gEfiRestJsonStructureProtocolGuid  = { 0xa9a048f6, 0x48a0, 0x4714,
> > {0xb7, 0xda, 0xa9, 0xad,0x87, 0xd4, 0xda, 0xc9 }}
> >
> > +  ## Include/Protocol/RedfishDiscover.h
> > +  gEfiRedfishDiscoverProtocolGuid      = { 0x5db12509, 0x4550, 0x4347,
> > { 0x96, 0xb3, 0x73, 0xc0, 0xff, 0x6e, 0x86, 0x9f }}
> > +
> >    #
> >    # Protocols defined in Shell2.0
> >    #
> > diff --git a/RedfishPkg/RedfishPkg.dec b/RedfishPkg/RedfishPkg.dec
> > index f12f83526c..65e21f6bd6 100644
> > --- a/RedfishPkg/RedfishPkg.dec
> > +++ b/RedfishPkg/RedfishPkg.dec
> > @@ -21,10 +21,6 @@
> >    #   Platform implementation-specific Redfish Host Interface.
> >
> > RedfishPlatformHostInterfaceLib|Include/Library/RedfishHostInterfaceLi
> > RedfishPlatformHostInterfaceLib|b.h
> >
> > -[Protocols]
> > -  ## Include/Protocol/RedfishDiscover.h
> > -  gEfiRedfishDiscoverProtocolGuid      = { 0x5db12509, 0x4550, 0x4347,
> > { 0x96, 0xb3, 0x73, 0xc0, 0xff, 0x6e, 0x86, 0x9f }}
> > -
> >  [Guids]
> >    gEfiRedfishPkgTokenSpaceGuid      = { 0x4fdbccb7, 0xe829, 0x4b4c,
> > { 0x88, 0x87, 0xb2, 0x3f, 0xd7, 0x25, 0x4b, 0x85 }}
> >
> > --
> > 2.17.1
> 
>