* [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
@ 2023-11-07 12:06 Igor Kulchytskyy via groups.io
2023-11-07 13:03 ` Nickle Wang via groups.io
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Igor Kulchytskyy via groups.io @ 2023-11-07 12:06 UTC (permalink / raw)
To: devel@edk2.groups.io; +Cc: Abner Chang, Nickle Wang
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.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Signed-off-by: Igor Kulchytskyy <igork@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 (#110847): https://edk2.groups.io/g/devel/message/110847
Mute This Topic: https://groups.io/mt/102441003/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-07 12:06 [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx Igor Kulchytskyy via groups.io
@ 2023-11-07 13:03 ` Nickle Wang via groups.io
2023-11-07 14:00 ` Mike Maslenkin
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Nickle Wang via groups.io @ 2023-11-07 13:03 UTC (permalink / raw)
To: Igor Kulchytskyy, devel@edk2.groups.io; +Cc: Abner Chang
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 <nicklew@nvidia.com>
Regards,
Nickle
> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Tuesday, November 7, 2023 8:06 PM
> To: devel@edk2.groups.io
> Cc: Abner Chang <abner.chang@amd.com>; Nickle Wang <nicklew@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.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <igork@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-07 12:06 [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx Igor Kulchytskyy via groups.io
2023-11-07 13:03 ` Nickle Wang via groups.io
@ 2023-11-07 14:00 ` Mike Maslenkin
2023-11-08 0:13 ` Chang, Abner via groups.io
2023-11-09 15:12 ` [edk2-devel] " Leif Lindholm
3 siblings, 0 replies; 10+ messages in thread
From: Mike Maslenkin @ 2023-11-07 14:00 UTC (permalink / raw)
To: devel, Igor Kulchytskyy; +Cc: Abner Chang, Nickle Wang
> On 7. 11. 2023., at 15:06, Igor Kulchytskyy via groups.io <igork=ami.com@groups.io> wrote:
>
> 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.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <igork@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.
Nice!
Ack.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110853): https://edk2.groups.io/g/devel/message/110853
Mute This Topic: https://groups.io/mt/102441003/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-07 12:06 [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx Igor Kulchytskyy via groups.io
2023-11-07 13:03 ` Nickle Wang via groups.io
2023-11-07 14:00 ` Mike Maslenkin
@ 2023-11-08 0:13 ` Chang, Abner via groups.io
2023-11-08 2:43 ` Nickle Wang via groups.io
2023-11-09 15:12 ` [edk2-devel] " Leif Lindholm
3 siblings, 1 reply; 10+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-08 0:13 UTC (permalink / raw)
To: Igor Kulchytskyy, devel@edk2.groups.io; +Cc: Nickle Wang
[AMD Official Use Only - General]
Reviewed-by: Abner Chang <abner.chang@amd.com>
> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Tuesday, November 7, 2023 8:06 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> <nicklew@nvidia.com>
> Subject: [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed
> after RestEx
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> 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.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <igork@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 (#110878): https://edk2.groups.io/g/devel/message/110878
Mute This Topic: https://groups.io/mt/102441003/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-08 0:13 ` Chang, Abner via groups.io
@ 2023-11-08 2:43 ` Nickle Wang via groups.io
2023-11-09 13:49 ` [edk2-devel] 回复: [edk2-stable202311] " gaoliming via groups.io
0 siblings, 1 reply; 10+ messages in thread
From: Nickle Wang via groups.io @ 2023-11-08 2:43 UTC (permalink / raw)
To: Liming Gao, Kinney, Michael D, devel@edk2.groups.io
Cc: Igor Kulchytskyy, Chang, Abner
[-- Attachment #1: Type: text/plain, Size: 17226 bytes --]
Hi @Liming Gao<mailto:gaoliming@byosoft.com.cn>, @Kinney, Michael D<mailto:michael.d.kinney@intel.com>
> If the patch is sent before Soft Feature Freeze, and plans to catch this stable tag, the patch contributor need reply to his patch and notify edk2 community
We would like to include this fix to edk2-stable202311 release. Could you please help us to merge this patch? This patch review was started on November 1: https://edk2.groups.io/g/devel/message/110440. Anber and I gave reviewed-by to this patch today. We have a pull request ready for merging here: https://github.com/tianocore/edk2/pull/4994
Thanks,
Nickle
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, Abner
> via groups.io
> Sent: Wednesday, November 8, 2023 8:14 AM
> To: Igor Kulchytskyy <igork@ami.com>; devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>
> Subject: Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if
> IPv4 installed after RestEx
>
> External email: Use caution opening links or attachments
>
>
> [AMD Official Use Only - General]
>
> Reviewed-by: Abner Chang <abner.chang@amd.com<mailto:abner.chang@amd.com>>
>
> > -----Original Message-----
> > From: Igor Kulchytskyy <igork@ami.com<mailto:igork@ami.com>>
> > Sent: Tuesday, November 7, 2023 8:06 PM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> > Cc: Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Nickle Wang
> > <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>
> > Subject: [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4
> > installed after RestEx
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > 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.chang@amd.com<mailto:abner.chang@amd.com>>
> > Cc: Nickle Wang <nicklew@nvidia.com<mailto:nicklew@nvidia.com>>
> > Signed-off-by: Igor Kulchytskyy <igork@ami.com<mailto:igork@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 (#110886): https://edk2.groups.io/g/devel/message/110886
Mute This Topic: https://groups.io/mt/102441003/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 42436 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [edk2-devel] 回复: [edk2-stable202311] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-08 2:43 ` Nickle Wang via groups.io
@ 2023-11-09 13:49 ` gaoliming via groups.io
2023-11-09 14:48 ` Leif Lindholm
0 siblings, 1 reply; 10+ messages in thread
From: gaoliming via groups.io @ 2023-11-09 13:49 UTC (permalink / raw)
To: devel, nicklew, 'Kinney, Michael D',
'Leif Lindholm', 'Andrew Fish'
Cc: 'Igor Kulchytskyy', 'Chang, Abner'
[-- Attachment #1: Type: text/plain, Size: 17963 bytes --]
Nickle:
This is a bug fix. Its impact is only in RedfishDiscoverDxe. I think it can
be merged for this stable tag.
Thanks
Liming
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Nickle Wang via
groups.io
发送时间: 2023年11月8日 10:44
收件人: Liming Gao <gaoliming@byosoft.com.cn>; Kinney, Michael D <michael.d.
kinney@intel.com>; devel@edk2.groups.io
抄送: Igor Kulchytskyy <igork@ami.com>; Chang, Abner <abner.chang@amd.com>
主题: Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue
if IPv4 installed after RestEx
Hi <mailto:gaoliming@byosoft.com.cn> @Liming Gao,
<mailto:michael.d.kinney@intel.com> @Kinney, Michael D
> If the patch is sent before Soft Feature Freeze, and plans to catch this
stable tag, the patch contributor need reply to his patch and notify edk2
community
We would like to include this fix to edk2-stable202311 release. Could you
please help us to merge this patch? This patch review was started on
November 1: https://edk2.groups.io/g/devel/message/110440. Anber and I gave
reviewed-by to this patch today. We have a pull request ready for merging
here: https://github.com/tianocore/edk2/pull/4994
Thanks,
Nickle
> -----Original Message-----
> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
<devel@edk2.groups.io <mailto:devel@edk2.groups.io> > On Behalf Of Chang,
Abner
> via groups.io
> Sent: Wednesday, November 8, 2023 8:14 AM
> To: Igor Kulchytskyy <igork@ami.com <mailto:igork@ami.com> >;
devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> Cc: Nickle Wang <nicklew@nvidia.com <mailto:nicklew@nvidia.com> >
> Subject: Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix
issue if
> IPv4 installed after RestEx
>
> External email: Use caution opening links or attachments
>
>
> [AMD Official Use Only - General]
>
> Reviewed-by: Abner Chang < <mailto:abner.chang@amd.com>
abner.chang@amd.com>
>
> > -----Original Message-----
> > From: Igor Kulchytskyy < <mailto:igork@ami.com> igork@ami.com>
> > Sent: Tuesday, November 7, 2023 8:06 PM
> > To: <mailto:devel@edk2.groups.io> devel@edk2.groups.io
> > Cc: Chang, Abner < <mailto:Abner.Chang@amd.com> Abner.Chang@amd.com>;
Nickle Wang
> > < <mailto:nicklew@nvidia.com> nicklew@nvidia.com>
> > Subject: [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4
> > installed after RestEx
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > 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 < <mailto:abner.chang@amd.com> abner.chang@amd.com>
> > Cc: Nickle Wang < <mailto:nicklew@nvidia.com> nicklew@nvidia.com>
> > Signed-off-by: Igor Kulchytskyy < <mailto:igork@ami.com> igork@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 (#110982): https://edk2.groups.io/g/devel/message/110982
Mute This Topic: https://groups.io/mt/102485712/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 58226 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] 回复: [edk2-stable202311] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-09 13:49 ` [edk2-devel] 回复: [edk2-stable202311] " gaoliming via groups.io
@ 2023-11-09 14:48 ` Leif Lindholm
0 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2023-11-09 14:48 UTC (permalink / raw)
To: devel, gaoliming, nicklew, 'Kinney, Michael D',
'Andrew Fish'
Cc: 'Igor Kulchytskyy', 'Chang, Abner'
(You have my old now-dysfunctional @nuviainc.com email address on cc,
please delete that one if you can.)
I think it's fine to merge this bugfix, but I have a couple of review
comments I will make on the v3 code itself, replying to that submission.
(I am however not the maintainer of RedFishPkg, so Nickle is free to
ignore me and push this version.)
/
Leif
On 2023-11-09 13:49, gaoliming via groups.io wrote:
> Nickle:
>
> This is a bug fix. Its impact is only in RedfishDiscoverDxe. I think
> it can be merged for this stable tag.
>
> Thanks
>
> Liming
>
> *发件人:*devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Nickle Wang
> via groups.io
> *发送时间:*2023年11月8日10:44
> *收件人:*Liming Gao <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; devel@edk2.groups.io
> *抄送:*Igor Kulchytskyy <igork@ami.com>; Chang, Abner <abner.chang@amd.com>
> *主题:*Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix
> issue if IPv4 installed after RestEx
>
> Hi @Liming Gao <mailto:gaoliming@byosoft.com.cn>, @Kinney, Michael D
> <mailto:michael.d.kinney@intel.com>
>
>> If the patch is sent before Soft Feature Freeze, and plans to catch this stable tag, the patch contributor need reply to his patch and notify edk2 community
>
> We would like to include this fix to edk2-stable202311 release. Could
> you please help us to merge this patch? This patch review was started on
> November 1: https://edk2.groups.io/g/devel/message/110440
> <https://edk2.groups.io/g/devel/message/110440>. Anber and I gave
> reviewed-by to this patch today. We have a pull request ready for
> merging here: https://github.com/tianocore/edk2/pull/4994
> <https://github.com/tianocore/edk2/pull/4994>
>
> Thanks,
>
> Nickle
>
>> -----Original Message-----
>
>> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io
> <mailto:devel@edk2.groups.io>> On Behalf Of Chang, Abner
>
>> via groups.io
>
>> Sent: Wednesday, November 8, 2023 8:14 AM
>
>> To: Igor Kulchytskyy <igork@ami.com <mailto:igork@ami.com>>; devel@edk2.groups.io
> <mailto:devel@edk2.groups.io>
>
>> Cc: Nickle Wang <nicklew@nvidia.com <mailto:nicklew@nvidia.com>>
>
>> Subject: Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if
>
>> IPv4 installed after RestEx
>
>>
>
>> External email: Use caution opening links or attachments
>
>>
>
>>
>
>> [AMD Official Use Only - General]
>
>>
>
>> Reviewed-by: Abner Chang <abner.chang@amd.com <mailto:abner.chang@amd.com>>
>
>>
>
>> > -----Original Message-----
>
>> > From: Igor Kulchytskyy <igork@ami.com <mailto:igork@ami.com>>
>
>> > Sent: Tuesday, November 7, 2023 8:06 PM
>
>> > To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>
>> > Cc: Chang, Abner <Abner.Chang@amd.com <mailto:Abner.Chang@amd.com>>; Nickle Wang
>
>> > <nicklew@nvidia.com <mailto:nicklew@nvidia.com>>
>
>> > Subject: [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4
>
>> > installed after RestEx
>
>> >
>
>> > Caution: This message originated from an External Source. Use proper
>
>> > caution when opening attachments, clicking links, or responding.
>
>> >
>
>> >
>
>> > 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.chang@amd.com <mailto:abner.chang@amd.com>>
>
>> > Cc: Nickle Wang <nicklew@nvidia.com <mailto:nicklew@nvidia.com>>
>
>> > Signed-off-by: Igor Kulchytskyy <igork@ami.com <mailto:igork@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 (#110988): https://edk2.groups.io/g/devel/message/110988
Mute This Topic: https://groups.io/mt/102485712/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-07 12:06 [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx Igor Kulchytskyy via groups.io
` (2 preceding siblings ...)
2023-11-08 0:13 ` Chang, Abner via groups.io
@ 2023-11-09 15:12 ` Leif Lindholm
2023-11-10 0:39 ` Chang, Abner via groups.io
3 siblings, 1 reply; 10+ messages in thread
From: Leif Lindholm @ 2023-11-09 15:12 UTC (permalink / raw)
To: devel, igork; +Cc: Abner Chang, Nickle Wang, gaoliming
On 2023-11-07 12:06, Igor Kulchytskyy via groups.io wrote:
> Supported function of the driver changed to wait for all newtwork
Typo:
newtwork ->
network
> interface to be installed.
> Filer out the network interfaces which are not supported by
Filer ->
Filter
> Redfish Host Interface.
These sound like two separate changes?
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Signed-off-by: Igor Kulchytskyy <igork@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))))
> + {
This could really benefit from some helper macros.
e.g. if the test could look like
if ((MAC_COMPARE (ThisNetworkInterface, TargetNetworkInterface) == 0) &&
(VALID_TCP6 (TargetNetworkInterface, ThisNetworkInterface) ||
VALID_TCP4 (TargetNetworkInterface, ThisNetworkInterface))) {
> 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
If this is just a local helper function, we probably want it STATIC.
> +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)))
Similarly, this becomes a bit of a handful.
But it also feels like the logic is incorrect?
I.e. it won't filter out entries where IpType ==
REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN, or invalid?
Shouldn't the test be
((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp4) &&
(IpType != REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4))
and
((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp6) &&
(IpType != REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)))
Either way, those tests could probably also do with being turned into
macros to improve readability.
/
Leif
> + {
> + 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 (#110990): https://edk2.groups.io/g/devel/message/110990
Mute This Topic: https://groups.io/mt/102441003/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-09 15:12 ` [edk2-devel] " Leif Lindholm
@ 2023-11-10 0:39 ` Chang, Abner via groups.io
2023-11-10 13:53 ` Leif Lindholm
0 siblings, 1 reply; 10+ messages in thread
From: Chang, Abner via groups.io @ 2023-11-10 0:39 UTC (permalink / raw)
To: Leif Lindholm, devel@edk2.groups.io, igork@ami.com
Cc: Nickle Wang, gaoliming@byosoft.com.cn
[AMD Official Use Only - General]
Thanks Leif, some responses are given in line.
As we would like to have this be part of edk2-stable202312, we prefer letting this change gets in stable release first and address the comment in another patch afterward.
Hi Igor,
Could you please check my comment and help to send another patch after edk2-stable202312?
Thanks
Abner
> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: Thursday, November 9, 2023 11:12 PM
> To: devel@edk2.groups.io; igork@ami.com
> Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> <nicklew@nvidia.com>; gaoliming@byosoft.com.cn
> Subject: Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix
> issue if IPv4 installed after RestEx
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 2023-11-07 12:06, Igor Kulchytskyy via groups.io wrote:
> > Supported function of the driver changed to wait for all newtwork
>
> Typo:
> newtwork ->
> network
>
> > interface to be installed.
> > Filer out the network interfaces which are not supported by
>
> Filer ->
> Filter
>
> > Redfish Host Interface.
>
> These sound like two separate changes?
This is for the same issue.
>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Signed-off-by: Igor Kulchytskyy <igork@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))))
> > + {
>
> This could really benefit from some helper macros.
> e.g. if the test could look like
Hi Igor,
Could we consider this suggestion after stable release?
>
> if ((MAC_COMPARE (ThisNetworkInterface, TargetNetworkInterface) == 0) &&
> (VALID_TCP6 (TargetNetworkInterface, ThisNetworkInterface) ||
> VALID_TCP4 (TargetNetworkInterface, ThisNetworkInterface))) {
>
> > 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
>
> If this is just a local helper function, we probably want it STATIC.
However, it is recommended that not using STATIC for local function for the source level debug purpose in 5.4.2.2.2 in edk2 C coding standard spec.
>
> > +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)))
>
> Similarly, this becomes a bit of a handful.
Igor, could we consider using MACRO for this as well?
>
> But it also feels like the logic is incorrect?
> I.e. it won't filter out entries where IpType ==
> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN, or
> invalid?
>
> Shouldn't the test be
>
> ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp4) &&
> (IpType != REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4))
>
> and
>
> ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp6) &&
> (IpType != REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)))
I think above is the error condition, thus there is a "continue" to skip this entry.
Thanks
Abner
>
> Either way, those tests could probably also do with being turned into
> macros to improve readability.
>
> /
> Leif
>
> > + {
> > + 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 (#111003): https://edk2.groups.io/g/devel/message/111003
Mute This Topic: https://groups.io/mt/102441003/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx
2023-11-10 0:39 ` Chang, Abner via groups.io
@ 2023-11-10 13:53 ` Leif Lindholm
0 siblings, 0 replies; 10+ messages in thread
From: Leif Lindholm @ 2023-11-10 13:53 UTC (permalink / raw)
To: Chang, Abner
Cc: devel@edk2.groups.io, igork@ami.com, Nickle Wang,
gaoliming@byosoft.com.cn
On Fri, Nov 10, 2023 at 00:39:53 +0000, Chang, Abner wrote:
> [AMD Official Use Only - General]
>
> Thanks Leif, some responses are given in line.
> As we would like to have this be part of edk2-stable202312, we
> prefer letting this change gets in stable release first and address
> the comment in another patch afterward.
>
> Hi Igor,
> Could you please check my comment and help to send another patch after edk2-stable202312?
>
> Thanks
> Abner
>
>
> > -----Original Message-----
> > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > Sent: Thursday, November 9, 2023 11:12 PM
> > To: devel@edk2.groups.io; igork@ami.com
> > Cc: Chang, Abner <Abner.Chang@amd.com>; Nickle Wang
> > <nicklew@nvidia.com>; gaoliming@byosoft.com.cn
> > Subject: Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix
> > issue if IPv4 installed after RestEx
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On 2023-11-07 12:06, Igor Kulchytskyy via groups.io wrote:
> > > Supported function of the driver changed to wait for all newtwork
> >
> > Typo:
> > newtwork ->
> > network
> >
> > > interface to be installed.
> > > Filer out the network interfaces which are not supported by
> >
> > Filer ->
> > Filter
> >
> > > Redfish Host Interface.
> >
> > These sound like two separate changes?
> This is for the same issue.
That means it should be in the same set, not in the same patch.
> > > Cc: Abner Chang <abner.chang@amd.com>
> > > Cc: Nickle Wang <nicklew@nvidia.com>
> > > Signed-off-by: Igor Kulchytskyy <igork@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))))
> > > + {
> >
> > This could really benefit from some helper macros.
> > e.g. if the test could look like
>
> Hi Igor,
> Could we consider this suggestion after stable release?
>
> >
> > if ((MAC_COMPARE (ThisNetworkInterface, TargetNetworkInterface) == 0) &&
> > (VALID_TCP6 (TargetNetworkInterface, ThisNetworkInterface) ||
> > VALID_TCP4 (TargetNetworkInterface, ThisNetworkInterface))) {
> >
> > > 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
> >
> > If this is just a local helper function, we probably want it STATIC.
> However, it is recommended that not using STATIC for local function
> for the source level debug purpose in 5.4.2.2.2 in edk2 C coding
> standard spec.
Yeah, we need to fix that doc. I have no idea what decade that comment
applied to, but I bet the year number started with a 1.
> > > +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)))
> >
> > Similarly, this becomes a bit of a handful.
> Igor, could we consider using MACRO for this as well?
>
> >
> > But it also feels like the logic is incorrect?
> > I.e. it won't filter out entries where IpType ==
> > REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN, or
> > invalid?
> >
> > Shouldn't the test be
> >
> > ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp4) &&
> > (IpType != REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4))
> >
> > and
> >
> > ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp6) &&
> > (IpType != REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)))
>
> I think above is the error condition, thus there is a "continue" to skip this entry.
Yes, but it's checking whether the error is one of two possible error
conditions instead of checking for whether the inputs are valid.
That's a bug.
/
Leif
> Thanks
> Abner
>
> >
> > Either way, those tests could probably also do with being turned into
> > macros to improve readability.
> >
> > /
> > Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111037): https://edk2.groups.io/g/devel/message/111037
Mute This Topic: https://groups.io/mt/102441003/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-11-10 13:53 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-07 12:06 [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx Igor Kulchytskyy via groups.io
2023-11-07 13:03 ` Nickle Wang via groups.io
2023-11-07 14:00 ` Mike Maslenkin
2023-11-08 0:13 ` Chang, Abner via groups.io
2023-11-08 2:43 ` Nickle Wang via groups.io
2023-11-09 13:49 ` [edk2-devel] 回复: [edk2-stable202311] " gaoliming via groups.io
2023-11-09 14:48 ` Leif Lindholm
2023-11-09 15:12 ` [edk2-devel] " Leif Lindholm
2023-11-10 0:39 ` Chang, Abner via groups.io
2023-11-10 13:53 ` Leif Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox