Thanks for fixing this issue. This version looks good to me. I test it on my
system and it works well.
Reviewed-by: Nickle Wang <nick...@nvidia.com>
Regards,
Nickle
> -----Original Message-----
> From: Igor Kulchytskyy <ig...@ami.com>
> Sent: Tuesday, November 7, 2023 8:06 PM
> To: devel@edk2.groups.io
> Cc: Abner Chang <abner.ch...@amd.com>; Nickle Wang <nick...@nvidia.com>
> Subject: [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4
> installed
> after RestEx
>
> External email: Use caution opening links or attachments
>
>
> Supported function of the driver changed to wait for all newtwork interface
> to be
> installed.
> Filer out the network interfaces which are not supported by Redfish Host
> Interface.
>
> Cc: Abner Chang <abner.ch...@amd.com>
> Cc: Nickle Wang <nick...@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <ig...@ami.com>
> ---
> RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 165 ++++++++++++++---
> ---
> 1 file changed, 117 insertions(+), 48 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 23da3b968f..85e47843e4 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -322,9 +322,16 @@ GetTargetNetworkInterfaceInternal ( {
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterface;
>
> + if (IsListEmpty (&mEfiRedfishDiscoverNetworkInterface)) {
> + return NULL;
> + }
> +
> ThisNetworkInterface =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> (&mEfiRedfishDiscoverNetworkInterface);
> while (TRUE) {
> - if (CompareMem ((VOID *)&ThisNetworkInterface->MacAddress,
> &TargetNetworkInterface->MacAddress, ThisNetworkInterface->HwAddressSize)
> == 0) {
> + if ((CompareMem ((VOID *)&ThisNetworkInterface->MacAddress,
> &TargetNetworkInterface->MacAddress, ThisNetworkInterface->HwAddressSize)
> == 0) &&
> + ((TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface-
> >NetworkProtocolType == ProtocolTypeTcp6)) ||
> + (!TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface-
> >NetworkProtocolType == ProtocolTypeTcp4))))
> + {
> return ThisNetworkInterface;
> }
>
> @@ -354,6 +361,10 @@ GetTargetNetworkInterfaceInternalByController ( {
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterface;
>
> + if (IsListEmpty (&mEfiRedfishDiscoverNetworkInterface)) {
> + return NULL;
> + }
> +
> ThisNetworkInterface =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> (&mEfiRedfishDiscoverNetworkInterface);
> while (TRUE) {
> if (ThisNetworkInterface->OpenDriverControllerHandle == ControllerHandle)
> { @@ -476,6 +487,42 @@ CheckIsIpVersion6 (
> return FALSE;
> }
>
> +/**
> + This function returns the IP type supported by the Host Interface.
> +
> + @retval 00h is Unknown
> + 01h is Ipv4
> + 02h is Ipv6
> +
> +**/
> +UINT8
> +GetHiIpProtocolType (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + REDFISH_OVER_IP_PROTOCOL_DATA *Data;
> + REDFISH_INTERFACE_DATA *DeviceDescriptor;
> +
> + Data = NULL;
> + DeviceDescriptor = NULL;
> + if (mSmbios == NULL) {
> + Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL, (VOID
> **)&mSmbios);
> + if (EFI_ERROR (Status)) {
> + return
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN;
> + }
> + }
> +
> + Status = RedfishGetHostInterfaceProtocolData (mSmbios,
> + &DeviceDescriptor, &Data); // Search for SMBIOS type 42h if (!EFI_ERROR
> (Status) && (Data != NULL) &&
> + (Data->HostIpAssignmentType == RedfishHostIpAssignmentStatic)) {
> + return Data->HostIpAddressFormat;
> + }
> +
> + return REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN;
> +}
> +
> /**
> This function discover Redfish service through SMBIOS host interface.
>
> @@ -512,6 +559,18 @@ DiscoverRedfishHostInterface (
>
> Status = RedfishGetHostInterfaceProtocolData (mSmbios, &DeviceDescriptor,
> &Data); // Search for SMBIOS type 42h
> if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
> + if ((Instance->NetworkInterface->NetworkProtocolType == ProtocolTypeTcp4)
> &&
> + (Data->HostIpAddressFormat !=
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4)) // IPv4 case
> + {
> + DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv4, but Host Interface
> requires Ipv6\n", __func__));
> + return EFI_UNSUPPORTED;
> + } else if ((Instance->NetworkInterface->NetworkProtocolType ==
> ProtocolTypeTcp6) &&
> + (Data->HostIpAddressFormat !=
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)) // IPv6 case
> + {
> + DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv6, but Host Interface
> requires IPv4\n", __func__));
> + return EFI_UNSUPPORTED;
> + }
> +
> //
> // Check if we can reach out Redfish service using this network
> interface.
> // Check with MAC address using Device Descriptor Data Device Type 04 and
> Type 05.
> @@ -1102,6 +1161,7 @@ RedfishServiceGetNetworkInterface (
> OUT EFI_REDFISH_DISCOVER_NETWORK_INTERFACE **NetworkIntfInstances
> )
> {
> + EFI_STATUS Status;
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL
> *ThisNetworkInterfaceIntn;
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *ThisNetworkInterface;
> EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
> @@ -1141,13 +1201,23 @@ RedfishServiceGetNetworkInterface (
>
> ThisNetworkInterfaceIntn =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> (&mEfiRedfishDiscoverNetworkInterface);
> while (TRUE) {
> + // If Get Subnet Info failed then skip this interface
> + Status = NetworkInterfaceGetSubnetInfo (ThisNetworkInterfaceIntn,
> ImageHandle); // Get subnet info
> + if (EFI_ERROR (Status)) {
> + if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface,
> &ThisNetworkInterfaceIntn->Entry)) {
> + break;
> + }
> +
> + ThisNetworkInterfaceIntn =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetNextNode
> (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry);
> + continue;
> + }
> +
> ThisNetworkInterface->IsIpv6 = FALSE;
> if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
> ThisNetworkInterface->IsIpv6 = TRUE;
> }
>
> CopyMem ((VOID *)&ThisNetworkInterface->MacAddress,
> &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn-
> >HwAddressSize);
> - NetworkInterfaceGetSubnetInfo (ThisNetworkInterfaceIntn, ImageHandle); //
> Get subnet info.
> if (!ThisNetworkInterface->IsIpv6) {
> IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4,
> &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
> } else {
> @@ -1230,7 +1300,12 @@ RedfishServiceAcquireService (
>
> if (TargetNetworkInterface != NULL) {
> TargetNetworkInterfaceInternal = GetTargetNetworkInterfaceInternal
> (TargetNetworkInterface);
> - NumNetworkInterfaces = 1;
> + if (TargetNetworkInterfaceInternal == NULL) {
> + DEBUG ((DEBUG_ERROR, "%a:No network interface on platform.\n",
> __func__));
> + return EFI_UNSUPPORTED;
> + }
> +
> + NumNetworkInterfaces = 1;
> } else {
> TargetNetworkInterfaceInternal =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetFirstNode
> (&mEfiRedfishDiscoverNetworkInterface);
> NumNetworkInterfaces = NumberOfNetworkInterface ();
> @@ -1260,7 +1335,13 @@ RedfishServiceAcquireService (
> // Get subnet information in case subnet information is not set because
> // RedfishServiceGetNetworkInterfaces hasn't been called yet.
> //
> - NetworkInterfaceGetSubnetInfo (TargetNetworkInterfaceInternal,
> ImageHandle);
> + Status1 = NetworkInterfaceGetSubnetInfo
> (TargetNetworkInterfaceInternal,
> ImageHandle);
> + if (EFI_ERROR (Status1)) {
> + DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n",
> __func__));
> + FreePool (Instance);
> + continue;
> + }
> +
> NewInstance = TRUE;
> }
>
> @@ -1547,25 +1628,26 @@ TestForRequiredProtocols (
> ControllerHandle,
> EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> );
> + if (EFI_ERROR (Status)) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + Status = gBS->OpenProtocol (
> + ControllerHandle,
> + gRequiredProtocol[Index].DiscoveredProtocolGuid,
> + (VOID **)&Id,
> + This->DriverBindingHandle,
> + ControllerHandle,
> + EFI_OPEN_PROTOCOL_GET_PROTOCOL
> + );
> if (!EFI_ERROR (Status)) {
> - Status = gBS->OpenProtocol (
> - ControllerHandle,
> - gRequiredProtocol[Index].DiscoveredProtocolGuid,
> - (VOID **)&Id,
> - This->DriverBindingHandle,
> - ControllerHandle,
> - EFI_OPEN_PROTOCOL_GET_PROTOCOL
> - );
> - if (EFI_ERROR (Status)) {
> - if (Index == ListCount - 1) {
> - DEBUG ((DEBUG_INFO, "%a: all required protocols are found on this
> controller handle: %p.\n", __func__, ControllerHandle));
> - return EFI_SUCCESS;
> - }
> - }
> + // Already installed
> + return EFI_UNSUPPORTED;
> }
> }
>
> - return EFI_UNSUPPORTED;
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: all required protocols are found on
> + this controller handle: %p.\n", __func__, ControllerHandle)); return
> + EFI_SUCCESS;
> }
>
> /**
> @@ -1600,10 +1682,24 @@ BuildupNetworkInterface (
> EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
> EFI_TPL OldTpl;
> BOOLEAN
> NewNetworkInterfaceInstalled;
> + UINT8 IpType;
> + UINTN ListCount;
>
> + ListCount = (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL));
> NewNetworkInterfaceInstalled = FALSE;
> Index = 0;
> - do {
> +
> + // Get IP Type to filter out unnecessary network protocol if possible
> + IpType = GetHiIpProtocolType ();
> +
> + for (Index = 0; Index < ListCount; Index++) {
> + // Check IP Type and skip an unnecessary network protocol if does not
> match
> + if (((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp4) &&
> (IpType
> == REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)) ||
> + ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp6) &&
> (IpType
> == REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4)))
> + {
> + continue;
> + }
> +
> Status = gBS->OpenProtocol (
> // Already in list?
> ControllerHandle,
> @@ -1614,11 +1710,6 @@ BuildupNetworkInterface (
> EFI_OPEN_PROTOCOL_GET_PROTOCOL
> );
> if (!EFI_ERROR (Status)) {
> - Index++;
> - if (Index == (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL))) {
> - break;
> - }
> -
> continue;
> }
>
> @@ -1631,11 +1722,6 @@ BuildupNetworkInterface (
> EFI_OPEN_PROTOCOL_GET_PROTOCOL
> );
> if (EFI_ERROR (Status)) {
> - Index++;
> - if (Index == (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL))) {
> - break;
> - }
> -
> continue;
> }
>
> @@ -1694,11 +1780,6 @@ BuildupNetworkInterface (
> ProtocolDiscoverIdPtr
> );
> if (EFI_ERROR (Status)) {
> - Index++;
> - if (Index == (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL))) {
> - break;
> - }
> -
> continue;
> }
>
> @@ -1755,25 +1836,13 @@ BuildupNetworkInterface (
> }
> } else {
> DEBUG ((DEBUG_MANAGEABILITY, "%a: Not REST EX, continue with
> next\n", __func__));
> - Index++;
> - if (Index == (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL))) {
> - break;
> - }
> -
> continue;
> }
> }
>
> return Status;
> - } else {
> - Index++;
> - if (Index == (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL))) {
> - break;
> - }
> -
> - continue;
> }
> - } while (Index < (sizeof (gRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL)));
> + }
>
> return EFI_DEVICE_ERROR;
> }
> --
> 2.37.1.windows.1
> -The information contained in this message may be confidential and proprietary
> to American Megatrends (AMI). This communication is intended to be read only
> by
> the individual or entity to whom it is addressed or by their designee. If the
> reader
> of this message is not the intended recipient, you are on notice that any
> distribution of this message, in any form, is strictly prohibited. Please
> promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then
> delete or destroy all copies of the transmission.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110849): https://edk2.groups.io/g/devel/message/110849
Mute This Topic: https://groups.io/mt/102441003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
