.../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 105 ++++++------------ 1 file changed, 37 insertions(+), 68 deletions(-)
From: Abner Chang <abner.chang@amd.com>
Remedy Redfish service discovery flow changes made
in commit 8736b8fd.
The above fix creates the dependency with SMBIOS 42h record,
which has a problem as SMBIOS 42h may not be created when
RedfishDiscovery.Supported() is invoked even all of the
required protocols are ready on the ControllerHandle. We can’t
guarantee SMBIOS 42 structure will be always created before
ConnectController(). USB NIC maybe detected late and it means
PlatformHostInterfaceBmcUsbNicLib can populate SMBIOS 42h
information late as well. Calling to
RedfishServiceGetNetworkInterface with the previous fix may
result in no network interface for BMC-exposed NIC as SMBIOS
42h is not ready yet.This is the first issue.
Second, to skip the network interface when
NetworkInterfaceGetSubnetInfo() returns a failure also has
problem, as the NIC may be configured via RestEx->Configure().
This happens after the Host interface is discovered, as at this
moment we have the sufficient network information to configure
BMC-exposed NIC.
Base on Redfish spec in 31.1.5.2, “EFI Redfish Client may provide
selection UI of network interfaces for Redfish service discovery.",
This means edk2 Redfish client gets all network interfaces
through RedfishServiceGetNetworkInterface and choose the desired
network interface at its discretion for Redfish service.
So the fix here is:
1. In BuildNetworkInterface(), we don’t skip any network
interface. In RedfishServiceGetNetworkInterface, we don’t
skip any network interface even the subnet information is not
retrieved. We will still return all of network interfaces to
client.
2. In RedfishServiceAcquireService for
EFI_REDFISH_RISCOVER_HOST_INTERFACE case, we don’t skip any
network interface even the subnet information is not
retrieved.
3. Added some more debug information.
Note: The subnet information is used for the scenario the system
is managed by a centralized Redfish service (not on BMC), says
the multiple Redfish computer system instances. As it mentions
in 31.1.5.2, Redfish client they may have to know the subnet
information so they can know the network domain the NIC is
connected. There may have multiple subnets in the corporation
network environment. So the subnet information provides client
an idea when they choose the network interface, so does VLAN ID.
Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
.../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 105 ++++++------------
1 file changed, 37 insertions(+), 68 deletions(-)
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 833ae2b969f..1cfcbcdc794 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -487,43 +487,6 @@ CheckIsIpVersion6 (
return FALSE;
}
-/**
- This function returns the IP type supported by the Host Interface.
-
- @retval 00h is Unknown
- 01h is Ipv4
- 02h is Ipv6
-
-**/
-STATIC
-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;
-}
-
/**
Check if Network Protocol Type matches with SMBIOS Type 42 IP Address Type.
@@ -583,7 +546,10 @@ DiscoverRedfishHostInterface (
}
Status = RedfishGetHostInterfaceProtocolData (mSmbios, &DeviceDescriptor, &Data); // Search for SMBIOS type 42h
- if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
+ if (EFI_ERROR (Status) || (Data == NULL) || (DeviceDescriptor == NULL)) {
+ DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is failed.\n", __func__));
+ return Status;
+ } else {
// Check IP Type and skip an unnecessary network protocol if does not match
if (FilterProtocol (Instance->NetworkInterface->NetworkProtocolType, Data->HostIpAddressFormat)) {
return EFI_UNSUPPORTED;
@@ -737,10 +703,7 @@ DiscoverRedfishHostInterface (
IsHttps
);
}
- } else {
- DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is failed.\n", __func__));
}
-
return Status;
}
@@ -891,6 +854,12 @@ AddAndSignalNewRedfishService (
DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n", DiscoveredInstance->Information.Location));
}
+ if (UseHttps) {
+ DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 443\n"));
+ } else {
+ DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 80\n"));
+ }
+
if (Uuid != NULL) {
DiscoveredInstance->Information.Uuid = (CHAR16 *)AllocatePool (AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
AsciiStrToUnicodeStrS ((const CHAR8 *)Uuid, DiscoveredInstance->Information.Uuid, AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
@@ -1015,9 +984,11 @@ AddAndSignalNewRedfishService (
(EFI_REST_EX_CONFIG_DATA)(UINT8 *)RestExHttpConfigData
);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "%a:REST EX configured..\n", __func__));
+ DEBUG ((DEBUG_ERROR, "%a: REST EX is not configured..\n", __func__));
DeleteRestEx = TRUE;
goto EXIT_FREE_ALL;
+ } else {
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: REST EX is configured..\n", __func__));
}
//
@@ -1207,6 +1178,8 @@ RedfishServiceGetNetworkInterface (
EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *ThisNetworkInterface;
EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
+
if ((This == NULL) || (NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs == NULL) ||
(ImageHandle == NULL))
{
@@ -1244,29 +1217,23 @@ RedfishServiceGetNetworkInterface (
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;
+ if (!EFI_ERROR (Status)) {
+ if (!ThisNetworkInterface->IsIpv6) {
+ IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4, &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
+ } else {
+ IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6, &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in IPv6 address information.
}
- ThisNetworkInterfaceIntn = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetNextNode (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry);
- continue;
+ ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn->SubnetPrefixLength;
}
+ CopyMem ((VOID *)&ThisNetworkInterface->MacAddress, &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn->HwAddressSize);
ThisNetworkInterface->IsIpv6 = FALSE;
if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
ThisNetworkInterface->IsIpv6 = TRUE;
}
- CopyMem ((VOID *)&ThisNetworkInterface->MacAddress, &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn->HwAddressSize);
- if (!ThisNetworkInterface->IsIpv6) {
- IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4, &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
- } else {
- IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6, &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in IPv6 address information.
- }
-
- ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn->SubnetPrefixLength;
- ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
+ ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
RestExInstance->NumberOfNetworkInterfaces++;
if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry)) {
break;
@@ -1378,9 +1345,16 @@ RedfishServiceAcquireService (
//
Status1 = NetworkInterfaceGetSubnetInfo (TargetNetworkInterfaceInternal, ImageHandle);
if (EFI_ERROR (Status1)) {
- DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n", __func__));
- FreePool (Instance);
- continue;
+ //
+ // Get subnet information could be failed for EFI_REDFISH_DISCOVER_HOST_INTERFACE case.
+ // We will configure network in AddAndSignalNewRedfishService. So don't skip this
+ // target network interface.
+ //
+ if ((Flags & EFI_REDFISH_DISCOVER_HOST_INTERFACE) == 0) {
+ DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n", __func__));
+ FreePool (Instance);
+ continue;
+ }
}
NewInstance = TRUE;
@@ -1448,6 +1422,7 @@ RedfishServiceAbortAcquire (
IN EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *TargetNetworkInterface OPTIONAL
)
{
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
// This function is used to abort Redfish service discovery through SSDP
// on the network interface. SSDP is optionally suppoted by EFI_REDFISH_DISCOVER_PROTOCOL,
// we dont have implementation for SSDP now.
@@ -1477,6 +1452,8 @@ RedfishServiceReleaseService (
EFI_REDFISH_DISCOVERED_INSTANCE *ThisRedfishInstance;
EFI_REDFISH_DISCOVERED_INTERNAL_LIST *DiscoveredRedfishInstance;
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
+
if (IsListEmpty (&mRedfishInstanceList)) {
DEBUG ((DEBUG_ERROR, "%a:No any discovered Redfish service.\n", __func__));
return EFI_NOT_FOUND;
@@ -1723,22 +1700,13 @@ BuildupNetworkInterface (
EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
EFI_TPL OldTpl;
BOOLEAN NewNetworkInterfaceInstalled;
- UINT8 IpType;
UINTN ListCount;
ListCount = (sizeof (mRequiredProtocol) / sizeof (REDFISH_DISCOVER_REQUIRED_PROTOCOL));
NewNetworkInterfaceInstalled = FALSE;
Index = 0;
- // 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 (FilterProtocol (mRequiredProtocol[Index].ProtocolType, IpType)) {
- continue;
- }
-
Status = gBS->OpenProtocol (
// Already in list?
ControllerHandle,
@@ -2164,6 +2132,7 @@ RedfishDiscoverDriverBindingStart (
IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
)
{
+ DEBUG ((DEBUG_MANAGEABILITY, "%a:Entry.\n", __func__));
return BuildupNetworkInterface (This, ControllerHandle);
}
--
2.37.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112128): https://edk2.groups.io/g/devel/message/112128
Mute This Topic: https://groups.io/mt/103014230/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-Hi Abner,
please see my comments below
On Wed, Dec 6, 2023 at 6:06 PM <abner.chang@amd.com> wrote:
>
> From: Abner Chang <abner.chang@amd.com>
>
> Remedy Redfish service discovery flow changes made
> in commit 8736b8fd.
>
> The above fix creates the dependency with SMBIOS 42h record,
> which has a problem as SMBIOS 42h may not be created when
> RedfishDiscovery.Supported() is invoked even all of the
> required protocols are ready on the ControllerHandle. We can’t
> guarantee SMBIOS 42 structure will be always created before
> ConnectController(). USB NIC maybe detected late and it means
> PlatformHostInterfaceBmcUsbNicLib can populate SMBIOS 42h
> information late as well. Calling to
> RedfishServiceGetNetworkInterface with the previous fix may
> result in no network interface for BMC-exposed NIC as SMBIOS
> 42h is not ready yet.This is the first issue.
>
> Second, to skip the network interface when
> NetworkInterfaceGetSubnetInfo() returns a failure also has
> problem, as the NIC may be configured via RestEx->Configure().
> This happens after the Host interface is discovered, as at this
> moment we have the sufficient network information to configure
> BMC-exposed NIC.
>
> Base on Redfish spec in 31.1.5.2, “EFI Redfish Client may provide
> selection UI of network interfaces for Redfish service discovery.",
> This means edk2 Redfish client gets all network interfaces
> through RedfishServiceGetNetworkInterface and choose the desired
> network interface at its discretion for Redfish service.
>
> So the fix here is:
> 1. In BuildNetworkInterface(), we don’t skip any network
> interface. In RedfishServiceGetNetworkInterface, we don’t
> skip any network interface even the subnet information is not
> retrieved. We will still return all of network interfaces to
> client.
> 2. In RedfishServiceAcquireService for
> EFI_REDFISH_RISCOVER_HOST_INTERFACE case, we don’t skip any
> network interface even the subnet information is not
> retrieved.
>
> 3. Added some more debug information.
>
> Note: The subnet information is used for the scenario the system
> is managed by a centralized Redfish service (not on BMC), says
> the multiple Redfish computer system instances. As it mentions
> in 31.1.5.2, Redfish client they may have to know the subnet
> information so they can know the network domain the NIC is
> connected. There may have multiple subnets in the corporation
> network environment. So the subnet information provides client
> an idea when they choose the network interface, so does VLAN ID.
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
> .../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 105 ++++++------------
> 1 file changed, 37 insertions(+), 68 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 833ae2b969f..1cfcbcdc794 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -487,43 +487,6 @@ CheckIsIpVersion6 (
> return FALSE;
> }
>
> -/**
> - This function returns the IP type supported by the Host Interface.
> -
> - @retval 00h is Unknown
> - 01h is Ipv4
> - 02h is Ipv6
> -
> -**/
> -STATIC
> -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;
> -}
> -
> /**
> Check if Network Protocol Type matches with SMBIOS Type 42 IP Address Type.
>
> @@ -583,7 +546,10 @@ DiscoverRedfishHostInterface (
> }
>
> Status = RedfishGetHostInterfaceProtocolData (mSmbios, &DeviceDescriptor, &Data); // Search for SMBIOS type 42h
> - if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
> + if (EFI_ERROR (Status) || (Data == NULL) || (DeviceDescriptor == NULL)) {
> + DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is failed.\n", __func__));
> + return Status;
> + } else {
> // Check IP Type and skip an unnecessary network protocol if does not match
> if (FilterProtocol (Instance->NetworkInterface->NetworkProtocolType, Data->HostIpAddressFormat)) {
> return EFI_UNSUPPORTED;
> @@ -737,10 +703,7 @@ DiscoverRedfishHostInterface (
> IsHttps
> );
> }
> - } else {
> - DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is failed.\n", __func__));
> }
> -
> return Status;
> }
>
> @@ -891,6 +854,12 @@ AddAndSignalNewRedfishService (
> DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n", DiscoveredInstance->Information.Location));
> }
>
> + if (UseHttps) {
> + DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 443\n"));
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 80\n"));
> + }
> +
Do we really need such trace?
It could be a bit misleading, because PcdRedfishServicePort was
introduced previously.
May be we could move this to the caller function
DiscoverRedfishHostInterface() where IsHttps was initialized?
> if (Uuid != NULL) {
> DiscoveredInstance->Information.Uuid = (CHAR16 *)AllocatePool (AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
> AsciiStrToUnicodeStrS ((const CHAR8 *)Uuid, DiscoveredInstance->Information.Uuid, AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
> @@ -1015,9 +984,11 @@ AddAndSignalNewRedfishService (
> (EFI_REST_EX_CONFIG_DATA)(UINT8 *)RestExHttpConfigData
> );
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "%a:REST EX configured..\n", __func__));
> + DEBUG ((DEBUG_ERROR, "%a: REST EX is not configured..\n", __func__));
> DeleteRestEx = TRUE;
> goto EXIT_FREE_ALL;
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: REST EX is configured..\n", __func__));
> }
>
> //
> @@ -1207,6 +1178,8 @@ RedfishServiceGetNetworkInterface (
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *ThisNetworkInterface;
> EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
>
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> +
> if ((This == NULL) || (NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs == NULL) ||
> (ImageHandle == NULL))
> {
> @@ -1244,29 +1217,23 @@ RedfishServiceGetNetworkInterface (
> 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;
> + if (!EFI_ERROR (Status)) {
> + if (!ThisNetworkInterface->IsIpv6) {
> + IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4, &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
> + } else {
> + IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6, &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in IPv6 address information.
> }
>
> - ThisNetworkInterfaceIntn = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetNextNode (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry);
> - continue;
> + ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn->SubnetPrefixLength;
> }
>
> + CopyMem ((VOID *)&ThisNetworkInterface->MacAddress, &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn->HwAddressSize);
> ThisNetworkInterface->IsIpv6 = FALSE;
> if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
> ThisNetworkInterface->IsIpv6 = TRUE;
> }
>
> - CopyMem ((VOID *)&ThisNetworkInterface->MacAddress, &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn->HwAddressSize);
> - if (!ThisNetworkInterface->IsIpv6) {
> - IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4, &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
> - } else {
> - IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6, &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in IPv6 address information.
> - }
> -
> - ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn->SubnetPrefixLength;
> - ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
> + ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
> RestExInstance->NumberOfNetworkInterfaces++;
> if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry)) {
> break;
As can be observed from this hunk the check " if
(!ThisNetworkInterface->IsIpv6) " was moved before the
ThisNetworkInterface->IsIpv6 initialization. Also I would consider
removing IsIpv6 at all.
ThisNetworkInterface->IsIpv6 depends on CheckIsIpVersion6() that is
(ThisNetworkInterface->NetworkProtocolType == ProtocolTypeTcp6),
so why do we need an additional variable with the same meaning?
ThisNetworkInterface->NetworkProtocolType is initialized early on
RedfishDiscoverDriverBindingStart()->BuildupNetworkInterface() path,
so it always has the correct value.
> @@ -1378,9 +1345,16 @@ RedfishServiceAcquireService (
> //
> Status1 = NetworkInterfaceGetSubnetInfo (TargetNetworkInterfaceInternal, ImageHandle);
> if (EFI_ERROR (Status1)) {
> - DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n", __func__));
> - FreePool (Instance);
> - continue;
> + //
> + // Get subnet information could be failed for EFI_REDFISH_DISCOVER_HOST_INTERFACE case.
> + // We will configure network in AddAndSignalNewRedfishService. So don't skip this
> + // target network interface.
> + //
> + if ((Flags & EFI_REDFISH_DISCOVER_HOST_INTERFACE) == 0) {
> + DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n", __func__));
> + FreePool (Instance);
> + continue;
> + }
> }
>
> NewInstance = TRUE;
> @@ -1448,6 +1422,7 @@ RedfishServiceAbortAcquire (
> IN EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *TargetNetworkInterface OPTIONAL
> )
> {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> // This function is used to abort Redfish service discovery through SSDP
> // on the network interface. SSDP is optionally suppoted by EFI_REDFISH_DISCOVER_PROTOCOL,
> // we dont have implementation for SSDP now.
> @@ -1477,6 +1452,8 @@ RedfishServiceReleaseService (
> EFI_REDFISH_DISCOVERED_INSTANCE *ThisRedfishInstance;
> EFI_REDFISH_DISCOVERED_INTERNAL_LIST *DiscoveredRedfishInstance;
>
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> +
> if (IsListEmpty (&mRedfishInstanceList)) {
> DEBUG ((DEBUG_ERROR, "%a:No any discovered Redfish service.\n", __func__));
> return EFI_NOT_FOUND;
> @@ -1723,22 +1700,13 @@ BuildupNetworkInterface (
> EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
> EFI_TPL OldTpl;
> BOOLEAN NewNetworkInterfaceInstalled;
> - UINT8 IpType;
> UINTN ListCount;
>
> ListCount = (sizeof (mRequiredProtocol) / sizeof (REDFISH_DISCOVER_REQUIRED_PROTOCOL));
> NewNetworkInterfaceInstalled = FALSE;
> Index = 0;
>
> - // 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 (FilterProtocol (mRequiredProtocol[Index].ProtocolType, IpType)) {
> - continue;
> - }
> -
> Status = gBS->OpenProtocol (
> // Already in list?
> ControllerHandle,
> @@ -2164,6 +2132,7 @@ RedfishDiscoverDriverBindingStart (
> IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> )
> {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a:Entry.\n", __func__));
> return BuildupNetworkInterface (This, ControllerHandle);
> }
>
> --
> 2.37.1.windows.1
>
I've tested this patch and it works for me.
Regards,
Mike.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112191): https://edk2.groups.io/g/devel/message/112191
Mute This Topic: https://groups.io/mt/103014230/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
[AMD Official Use Only - General]
Hi Mike, below is my answer,
> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Thursday, December 7, 2023 9:37 PM
> To: Chang, Abner <Abner.Chang@amd.com>
> Cc: devel@edk2.groups.io; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: Re: [PATCH] RedfishPkg/RedfishDicovery: Remedy Redfish service
> discovery flow
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> please see my comments below
>
> On Wed, Dec 6, 2023 at 6:06 PM <abner.chang@amd.com> wrote:
> >
> > From: Abner Chang <abner.chang@amd.com>
> >
> > Remedy Redfish service discovery flow changes made
> > in commit 8736b8fd.
> >
> > The above fix creates the dependency with SMBIOS 42h record,
> > which has a problem as SMBIOS 42h may not be created when
> > RedfishDiscovery.Supported() is invoked even all of the
> > required protocols are ready on the ControllerHandle. We can’t
> > guarantee SMBIOS 42 structure will be always created before
> > ConnectController(). USB NIC maybe detected late and it means
> > PlatformHostInterfaceBmcUsbNicLib can populate SMBIOS 42h
> > information late as well. Calling to
> > RedfishServiceGetNetworkInterface with the previous fix may
> > result in no network interface for BMC-exposed NIC as SMBIOS
> > 42h is not ready yet.This is the first issue.
> >
> > Second, to skip the network interface when
> > NetworkInterfaceGetSubnetInfo() returns a failure also has
> > problem, as the NIC may be configured via RestEx->Configure().
> > This happens after the Host interface is discovered, as at this
> > moment we have the sufficient network information to configure
> > BMC-exposed NIC.
> >
> > Base on Redfish spec in 31.1.5.2, “EFI Redfish Client may provide
> > selection UI of network interfaces for Redfish service discovery.",
> > This means edk2 Redfish client gets all network interfaces
> > through RedfishServiceGetNetworkInterface and choose the desired
> > network interface at its discretion for Redfish service.
> >
> > So the fix here is:
> > 1. In BuildNetworkInterface(), we don’t skip any network
> > interface. In RedfishServiceGetNetworkInterface, we don’t
> > skip any network interface even the subnet information is not
> > retrieved. We will still return all of network interfaces to
> > client.
> > 2. In RedfishServiceAcquireService for
> > EFI_REDFISH_RISCOVER_HOST_INTERFACE case, we don’t skip any
> > network interface even the subnet information is not
> > retrieved.
> >
> > 3. Added some more debug information.
> >
> > Note: The subnet information is used for the scenario the system
> > is managed by a centralized Redfish service (not on BMC), says
> > the multiple Redfish computer system instances. As it mentions
> > in 31.1.5.2, Redfish client they may have to know the subnet
> > information so they can know the network domain the NIC is
> > connected. There may have multiple subnets in the corporation
> > network environment. So the subnet information provides client
> > an idea when they choose the network interface, so does VLAN ID.
> >
> > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Igor Kulchytskyy <igork@ami.com>
> > Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> > ---
> > .../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 105 ++++++------------
> > 1 file changed, 37 insertions(+), 68 deletions(-)
> >
> > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > index 833ae2b969f..1cfcbcdc794 100644
> > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> > @@ -487,43 +487,6 @@ CheckIsIpVersion6 (
> > return FALSE;
> > }
> >
> > -/**
> > - This function returns the IP type supported by the Host Interface.
> > -
> > - @retval 00h is Unknown
> > - 01h is Ipv4
> > - 02h is Ipv6
> > -
> > -**/
> > -STATIC
> > -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;
> > -}
> > -
> > /**
> > Check if Network Protocol Type matches with SMBIOS Type 42 IP Address
> Type.
> >
> > @@ -583,7 +546,10 @@ DiscoverRedfishHostInterface (
> > }
> >
> > Status = RedfishGetHostInterfaceProtocolData (mSmbios,
> &DeviceDescriptor, &Data); // Search for SMBIOS type 42h
> > - if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL))
> {
> > + if (EFI_ERROR (Status) || (Data == NULL) || (DeviceDescriptor == NULL)) {
> > + DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is
> failed.\n", __func__));
> > + return Status;
> > + } else {
> > // Check IP Type and skip an unnecessary network protocol if does not
> match
> > if (FilterProtocol (Instance->NetworkInterface->NetworkProtocolType,
> Data->HostIpAddressFormat)) {
> > return EFI_UNSUPPORTED;
> > @@ -737,10 +703,7 @@ DiscoverRedfishHostInterface (
> > IsHttps
> > );
> > }
> > - } else {
> > - DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is
> failed.\n", __func__));
> > }
> > -
> > return Status;
> > }
> >
> > @@ -891,6 +854,12 @@ AddAndSignalNewRedfishService (
> > DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n",
> DiscoveredInstance->Information.Location));
> > }
> >
> > + if (UseHttps) {
> > + DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 443\n"));
> > + } else {
> > + DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 80\n"));
> > + }
> > +
>
> Do we really need such trace?
> It could be a bit misleading, because PcdRedfishServicePort was
> introduced previously.
> May be we could move this to the caller function
> DiscoverRedfishHostInterface() where IsHttps was initialized?
Yes, move this to DiscoverRedfishHostInterface makes more sense to me as well.
>
>
> > if (Uuid != NULL) {
> > DiscoveredInstance->Information.Uuid = (CHAR16 *)AllocatePool
> (AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
> > AsciiStrToUnicodeStrS ((const CHAR8 *)Uuid, DiscoveredInstance-
> >Information.Uuid, AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
> > @@ -1015,9 +984,11 @@ AddAndSignalNewRedfishService (
> > (EFI_REST_EX_CONFIG_DATA)(UINT8 *)RestExHttpConfigData
> > );
> > if (EFI_ERROR (Status)) {
> > - DEBUG ((DEBUG_ERROR, "%a:REST EX configured..\n", __func__));
> > + DEBUG ((DEBUG_ERROR, "%a: REST EX is not configured..\n",
> __func__));
> > DeleteRestEx = TRUE;
> > goto EXIT_FREE_ALL;
> > + } else {
> > + DEBUG ((DEBUG_MANAGEABILITY, "%a: REST EX is configured..\n",
> __func__));
> > }
> >
> > //
> > @@ -1207,6 +1178,8 @@ RedfishServiceGetNetworkInterface (
> > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *ThisNetworkInterface;
> > EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
> >
> > + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> > +
> > if ((This == NULL) || (NetworkIntfInstances == NULL) ||
> (NumberOfNetworkIntfs == NULL) ||
> > (ImageHandle == NULL))
> > {
> > @@ -1244,29 +1217,23 @@ RedfishServiceGetNetworkInterface (
> > 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;
> > + if (!EFI_ERROR (Status)) {
> > + if (!ThisNetworkInterface->IsIpv6) {
> > + IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4,
> &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
> > + } else {
> > + IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6,
> &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in
> IPv6 address information.
> > }
> >
> > - ThisNetworkInterfaceIntn =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetNextNode
> (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry);
> > - continue;
> > + ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn-
> >SubnetPrefixLength;
> > }
> >
> > + CopyMem ((VOID *)&ThisNetworkInterface->MacAddress,
> &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn-
> >HwAddressSize);
> > ThisNetworkInterface->IsIpv6 = FALSE;
> > if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
> > ThisNetworkInterface->IsIpv6 = TRUE;
> > }
> >
> > - CopyMem ((VOID *)&ThisNetworkInterface->MacAddress,
> &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn-
> >HwAddressSize);
> > - if (!ThisNetworkInterface->IsIpv6) {
> > - IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4,
> &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
> > - } else {
> > - IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6,
> &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in
> IPv6 address information.
> > - }
> > -
> > - ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn-
> >SubnetPrefixLength;
> > - ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
> > + ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
> > RestExInstance->NumberOfNetworkInterfaces++;
> > if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface,
> &ThisNetworkInterfaceIntn->Entry)) {
> > break;
>
> As can be observed from this hunk the check " if
> (!ThisNetworkInterface->IsIpv6) " was moved before the
> ThisNetworkInterface->IsIpv6 initialization. Also I would consider
> removing IsIpv6 at all.
> ThisNetworkInterface->IsIpv6 depends on CheckIsIpVersion6() that is
> (ThisNetworkInterface->NetworkProtocolType == ProtocolTypeTcp6),
> so why do we need an additional variable with the same meaning?
> ThisNetworkInterface->NetworkProtocolType is initialized early on
> RedfishDiscoverDriverBindingStart()->BuildupNetworkInterface() path,
> so it always has the correct value.
This is a mistake when I move around the code, we should set ThisNetworkInterface->IsIpv6 before copying subnet ID. We don’t encounter the problem because ThisNetworkInterface->IsIpv6 is initiated as 0 when AllocateZeroPool and we don’t have the Redfish service in IPv6 case, I guess.
Regarding the two variables with same meaning, one is for the output data according to the structure defined in spec (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE ) and another one is the internal record of network interface.
I will submit the next version if this makes sense to you.
Thanks
Abner
>
>
> > @@ -1378,9 +1345,16 @@ RedfishServiceAcquireService (
> > //
> > Status1 = NetworkInterfaceGetSubnetInfo
> (TargetNetworkInterfaceInternal, ImageHandle);
> > if (EFI_ERROR (Status1)) {
> > - DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n",
> __func__));
> > - FreePool (Instance);
> > - continue;
> > + //
> > + // Get subnet information could be failed for
> EFI_REDFISH_DISCOVER_HOST_INTERFACE case.
> > + // We will configure network in AddAndSignalNewRedfishService. So
> don't skip this
> > + // target network interface.
> > + //
> > + if ((Flags & EFI_REDFISH_DISCOVER_HOST_INTERFACE) == 0) {
> > + DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n",
> __func__));
> > + FreePool (Instance);
> > + continue;
> > + }
> > }
> >
> > NewInstance = TRUE;
> > @@ -1448,6 +1422,7 @@ RedfishServiceAbortAcquire (
> > IN EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *TargetNetworkInterface OPTIONAL
> > )
> > {
> > + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> > // This function is used to abort Redfish service discovery through SSDP
> > // on the network interface. SSDP is optionally suppoted by
> EFI_REDFISH_DISCOVER_PROTOCOL,
> > // we dont have implementation for SSDP now.
> > @@ -1477,6 +1452,8 @@ RedfishServiceReleaseService (
> > EFI_REDFISH_DISCOVERED_INSTANCE *ThisRedfishInstance;
> > EFI_REDFISH_DISCOVERED_INTERNAL_LIST *DiscoveredRedfishInstance;
> >
> > + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> > +
> > if (IsListEmpty (&mRedfishInstanceList)) {
> > DEBUG ((DEBUG_ERROR, "%a:No any discovered Redfish service.\n",
> __func__));
> > return EFI_NOT_FOUND;
> > @@ -1723,22 +1700,13 @@ BuildupNetworkInterface (
> > EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
> > EFI_TPL OldTpl;
> > BOOLEAN NewNetworkInterfaceInstalled;
> > - UINT8 IpType;
> > UINTN ListCount;
> >
> > ListCount = (sizeof (mRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL));
> > NewNetworkInterfaceInstalled = FALSE;
> > Index = 0;
> >
> > - // 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 (FilterProtocol (mRequiredProtocol[Index].ProtocolType, IpType)) {
> > - continue;
> > - }
> > -
> > Status = gBS->OpenProtocol (
> > // Already in list?
> > ControllerHandle,
> > @@ -2164,6 +2132,7 @@ RedfishDiscoverDriverBindingStart (
> > IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> > )
> > {
> > + DEBUG ((DEBUG_MANAGEABILITY, "%a:Entry.\n", __func__));
> > return BuildupNetworkInterface (This, ControllerHandle);
> > }
> >
> > --
> > 2.37.1.windows.1
> >
>
> I've tested this patch and it works for me.
>
> Regards,
> Mike.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112212): https://edk2.groups.io/g/devel/message/112212
Mute This Topic: https://groups.io/mt/103014230/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Igor Kulchytskyy <igork@ami.com>
Regards,
Igor
-----Original Message-----
From: abner.chang@amd.com <abner.chang@amd.com>
Sent: Wednesday, December 6, 2023 10:05 AM
To: devel@edk2.groups.io
Cc: Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>; Mike Maslenkin <mike.maslenkin@gmail.com>
Subject: [EXTERNAL] [PATCH] RedfishPkg/RedfishDicovery: Remedy Redfish service discovery flow
**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**
From: Abner Chang <abner.chang@amd.com>
Remedy Redfish service discovery flow changes made
in commit 8736b8fd.
The above fix creates the dependency with SMBIOS 42h record,
which has a problem as SMBIOS 42h may not be created when
RedfishDiscovery.Supported() is invoked even all of the
required protocols are ready on the ControllerHandle. We can’t
guarantee SMBIOS 42 structure will be always created before
ConnectController(). USB NIC maybe detected late and it means
PlatformHostInterfaceBmcUsbNicLib can populate SMBIOS 42h
information late as well. Calling to
RedfishServiceGetNetworkInterface with the previous fix may
result in no network interface for BMC-exposed NIC as SMBIOS
42h is not ready yet.This is the first issue.
Second, to skip the network interface when
NetworkInterfaceGetSubnetInfo() returns a failure also has
problem, as the NIC may be configured via RestEx->Configure().
This happens after the Host interface is discovered, as at this
moment we have the sufficient network information to configure
BMC-exposed NIC.
Base on Redfish spec in 31.1.5.2, “EFI Redfish Client may provide
selection UI of network interfaces for Redfish service discovery.",
This means edk2 Redfish client gets all network interfaces
through RedfishServiceGetNetworkInterface and choose the desired
network interface at its discretion for Redfish service.
So the fix here is:
1. In BuildNetworkInterface(), we don’t skip any network
interface. In RedfishServiceGetNetworkInterface, we don’t
skip any network interface even the subnet information is not
retrieved. We will still return all of network interfaces to
client.
2. In RedfishServiceAcquireService for
EFI_REDFISH_RISCOVER_HOST_INTERFACE case, we don’t skip any
network interface even the subnet information is not
retrieved.
3. Added some more debug information.
Note: The subnet information is used for the scenario the system
is managed by a centralized Redfish service (not on BMC), says
the multiple Redfish computer system instances. As it mentions
in 31.1.5.2, Redfish client they may have to know the subnet
information so they can know the network domain the NIC is
connected. There may have multiple subnets in the corporation
network environment. So the subnet information provides client
an idea when they choose the network interface, so does VLAN ID.
Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
.../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 105 ++++++------------
1 file changed, 37 insertions(+), 68 deletions(-)
diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 833ae2b969f..1cfcbcdc794 100644
--- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
+++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
@@ -487,43 +487,6 @@ CheckIsIpVersion6 (
return FALSE;
}
-/**
- This function returns the IP type supported by the Host Interface.
-
- @retval 00h is Unknown
- 01h is Ipv4
- 02h is Ipv6
-
-**/
-STATIC
-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;
-}
-
/**
Check if Network Protocol Type matches with SMBIOS Type 42 IP Address Type.
@@ -583,7 +546,10 @@ DiscoverRedfishHostInterface (
}
Status = RedfishGetHostInterfaceProtocolData (mSmbios, &DeviceDescriptor, &Data); // Search for SMBIOS type 42h
- if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
+ if (EFI_ERROR (Status) || (Data == NULL) || (DeviceDescriptor == NULL)) {
+ DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is failed.\n", __func__));
+ return Status;
+ } else {
// Check IP Type and skip an unnecessary network protocol if does not match
if (FilterProtocol (Instance->NetworkInterface->NetworkProtocolType, Data->HostIpAddressFormat)) {
return EFI_UNSUPPORTED;
@@ -737,10 +703,7 @@ DiscoverRedfishHostInterface (
IsHttps
);
}
- } else {
- DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is failed.\n", __func__));
}
-
return Status;
}
@@ -891,6 +854,12 @@ AddAndSignalNewRedfishService (
DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n", DiscoveredInstance->Information.Location));
}
+ if (UseHttps) {
+ DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 443\n"));
+ } else {
+ DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 80\n"));
+ }
+
if (Uuid != NULL) {
DiscoveredInstance->Information.Uuid = (CHAR16 *)AllocatePool (AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
AsciiStrToUnicodeStrS ((const CHAR8 *)Uuid, DiscoveredInstance->Information.Uuid, AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
@@ -1015,9 +984,11 @@ AddAndSignalNewRedfishService (
(EFI_REST_EX_CONFIG_DATA)(UINT8 *)RestExHttpConfigData
);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "%a:REST EX configured..\n", __func__));
+ DEBUG ((DEBUG_ERROR, "%a: REST EX is not configured..\n", __func__));
DeleteRestEx = TRUE;
goto EXIT_FREE_ALL;
+ } else {
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: REST EX is configured..\n", __func__));
}
//
@@ -1207,6 +1178,8 @@ RedfishServiceGetNetworkInterface (
EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *ThisNetworkInterface;
EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
+
if ((This == NULL) || (NetworkIntfInstances == NULL) || (NumberOfNetworkIntfs == NULL) ||
(ImageHandle == NULL))
{
@@ -1244,29 +1217,23 @@ RedfishServiceGetNetworkInterface (
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;
+ if (!EFI_ERROR (Status)) {
+ if (!ThisNetworkInterface->IsIpv6) {
+ IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4, &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
+ } else {
+ IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6, &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in IPv6 address information.
}
- ThisNetworkInterfaceIntn = (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetNextNode (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry);
- continue;
+ ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn->SubnetPrefixLength;
}
+ CopyMem ((VOID *)&ThisNetworkInterface->MacAddress, &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn->HwAddressSize);
ThisNetworkInterface->IsIpv6 = FALSE;
if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
ThisNetworkInterface->IsIpv6 = TRUE;
}
- CopyMem ((VOID *)&ThisNetworkInterface->MacAddress, &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn->HwAddressSize);
- if (!ThisNetworkInterface->IsIpv6) {
- IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4, &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
- } else {
- IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6, &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in IPv6 address information.
- }
-
- ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn->SubnetPrefixLength;
- ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
+ ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
RestExInstance->NumberOfNetworkInterfaces++;
if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry)) {
break;
@@ -1378,9 +1345,16 @@ RedfishServiceAcquireService (
//
Status1 = NetworkInterfaceGetSubnetInfo (TargetNetworkInterfaceInternal, ImageHandle);
if (EFI_ERROR (Status1)) {
- DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n", __func__));
- FreePool (Instance);
- continue;
+ //
+ // Get subnet information could be failed for EFI_REDFISH_DISCOVER_HOST_INTERFACE case.
+ // We will configure network in AddAndSignalNewRedfishService. So don't skip this
+ // target network interface.
+ //
+ if ((Flags & EFI_REDFISH_DISCOVER_HOST_INTERFACE) == 0) {
+ DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n", __func__));
+ FreePool (Instance);
+ continue;
+ }
}
NewInstance = TRUE;
@@ -1448,6 +1422,7 @@ RedfishServiceAbortAcquire (
IN EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *TargetNetworkInterface OPTIONAL
)
{
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
// This function is used to abort Redfish service discovery through SSDP
// on the network interface. SSDP is optionally suppoted by EFI_REDFISH_DISCOVER_PROTOCOL,
// we dont have implementation for SSDP now.
@@ -1477,6 +1452,8 @@ RedfishServiceReleaseService (
EFI_REDFISH_DISCOVERED_INSTANCE *ThisRedfishInstance;
EFI_REDFISH_DISCOVERED_INTERNAL_LIST *DiscoveredRedfishInstance;
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
+
if (IsListEmpty (&mRedfishInstanceList)) {
DEBUG ((DEBUG_ERROR, "%a:No any discovered Redfish service.\n", __func__));
return EFI_NOT_FOUND;
@@ -1723,22 +1700,13 @@ BuildupNetworkInterface (
EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
EFI_TPL OldTpl;
BOOLEAN NewNetworkInterfaceInstalled;
- UINT8 IpType;
UINTN ListCount;
ListCount = (sizeof (mRequiredProtocol) / sizeof (REDFISH_DISCOVER_REQUIRED_PROTOCOL));
NewNetworkInterfaceInstalled = FALSE;
Index = 0;
- // 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 (FilterProtocol (mRequiredProtocol[Index].ProtocolType, IpType)) {
- continue;
- }
-
Status = gBS->OpenProtocol (
// Already in list?
ControllerHandle,
@@ -2164,6 +2132,7 @@ RedfishDiscoverDriverBindingStart (
IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
)
{
+ DEBUG ((DEBUG_MANAGEABILITY, "%a:Entry.\n", __func__));
return BuildupNetworkInterface (This, ControllerHandle);
}
--
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 (#112188): https://edk2.groups.io/g/devel/message/112188
Mute This Topic: https://groups.io/mt/103014230/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
[AMD Official Use Only - General]
Hi Igor, thanks for testing this at your end.
Abner
> -----Original Message-----
> From: Igor Kulchytskyy <igork@ami.com>
> Sent: Thursday, December 7, 2023 9:23 PM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>; Mike Maslenkin
> <mike.maslenkin@gmail.com>
> Subject: RE: [EXTERNAL] [PATCH] RedfishPkg/RedfishDicovery: Remedy
> Redfish service discovery flow
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Reviewed-by: Igor Kulchytskyy <igork@ami.com>
> Regards,
> Igor
>
> -----Original Message-----
> From: abner.chang@amd.com <abner.chang@amd.com>
> Sent: Wednesday, December 6, 2023 10:05 AM
> To: devel@edk2.groups.io
> Cc: Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>;
> Mike Maslenkin <mike.maslenkin@gmail.com>
> Subject: [EXTERNAL] [PATCH] RedfishPkg/RedfishDicovery: Remedy Redfish
> service discovery flow
>
>
> **CAUTION: The e-mail below is from an external source. Please exercise
> caution before opening attachments, clicking links, or following guidance.**
>
> From: Abner Chang <abner.chang@amd.com>
>
> Remedy Redfish service discovery flow changes made
> in commit 8736b8fd.
>
> The above fix creates the dependency with SMBIOS 42h record,
> which has a problem as SMBIOS 42h may not be created when
> RedfishDiscovery.Supported() is invoked even all of the
> required protocols are ready on the ControllerHandle. We can’t
> guarantee SMBIOS 42 structure will be always created before
> ConnectController(). USB NIC maybe detected late and it means
> PlatformHostInterfaceBmcUsbNicLib can populate SMBIOS 42h
> information late as well. Calling to
> RedfishServiceGetNetworkInterface with the previous fix may
> result in no network interface for BMC-exposed NIC as SMBIOS
> 42h is not ready yet.This is the first issue.
>
> Second, to skip the network interface when
> NetworkInterfaceGetSubnetInfo() returns a failure also has
> problem, as the NIC may be configured via RestEx->Configure().
> This happens after the Host interface is discovered, as at this
> moment we have the sufficient network information to configure
> BMC-exposed NIC.
>
> Base on Redfish spec in 31.1.5.2, “EFI Redfish Client may provide
> selection UI of network interfaces for Redfish service discovery.",
> This means edk2 Redfish client gets all network interfaces
> through RedfishServiceGetNetworkInterface and choose the desired
> network interface at its discretion for Redfish service.
>
> So the fix here is:
> 1. In BuildNetworkInterface(), we don’t skip any network
> interface. In RedfishServiceGetNetworkInterface, we don’t
> skip any network interface even the subnet information is not
> retrieved. We will still return all of network interfaces to
> client.
> 2. In RedfishServiceAcquireService for
> EFI_REDFISH_RISCOVER_HOST_INTERFACE case, we don’t skip any
> network interface even the subnet information is not
> retrieved.
>
> 3. Added some more debug information.
>
> Note: The subnet information is used for the scenario the system
> is managed by a centralized Redfish service (not on BMC), says
> the multiple Redfish computer system instances. As it mentions
> in 31.1.5.2, Redfish client they may have to know the subnet
> information so they can know the network domain the NIC is
> connected. There may have multiple subnets in the corporation
> network environment. So the subnet information provides client
> an idea when they choose the network interface, so does VLAN ID.
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
> ---
> .../RedfishDiscoverDxe/RedfishDiscoverDxe.c | 105 ++++++------------
> 1 file changed, 37 insertions(+), 68 deletions(-)
>
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> index 833ae2b969f..1cfcbcdc794 100644
> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
> @@ -487,43 +487,6 @@ CheckIsIpVersion6 (
> return FALSE;
> }
>
> -/**
> - This function returns the IP type supported by the Host Interface.
> -
> - @retval 00h is Unknown
> - 01h is Ipv4
> - 02h is Ipv6
> -
> -**/
> -STATIC
> -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;
> -}
> -
> /**
> Check if Network Protocol Type matches with SMBIOS Type 42 IP Address
> Type.
>
> @@ -583,7 +546,10 @@ DiscoverRedfishHostInterface (
> }
>
> Status = RedfishGetHostInterfaceProtocolData (mSmbios,
> &DeviceDescriptor, &Data); // Search for SMBIOS type 42h
> - if (!EFI_ERROR (Status) && (Data != NULL) && (DeviceDescriptor != NULL)) {
> + if (EFI_ERROR (Status) || (Data == NULL) || (DeviceDescriptor == NULL)) {
> + DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is
> failed.\n", __func__));
> + return Status;
> + } else {
> // Check IP Type and skip an unnecessary network protocol if does not
> match
> if (FilterProtocol (Instance->NetworkInterface->NetworkProtocolType,
> Data->HostIpAddressFormat)) {
> return EFI_UNSUPPORTED;
> @@ -737,10 +703,7 @@ DiscoverRedfishHostInterface (
> IsHttps
> );
> }
> - } else {
> - DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is
> failed.\n", __func__));
> }
> -
> return Status;
> }
>
> @@ -891,6 +854,12 @@ AddAndSignalNewRedfishService (
> DEBUG ((DEBUG_MANAGEABILITY, "Redfish service location: %s.\n",
> DiscoveredInstance->Information.Location));
> }
>
> + if (UseHttps) {
> + DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 443\n"));
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 80\n"));
> + }
> +
> if (Uuid != NULL) {
> DiscoveredInstance->Information.Uuid = (CHAR16 *)AllocatePool
> (AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
> AsciiStrToUnicodeStrS ((const CHAR8 *)Uuid, DiscoveredInstance-
> >Information.Uuid, AsciiStrSize ((const CHAR8 *)Uuid) * sizeof (CHAR16));
> @@ -1015,9 +984,11 @@ AddAndSignalNewRedfishService (
> (EFI_REST_EX_CONFIG_DATA)(UINT8 *)RestExHttpConfigData
> );
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "%a:REST EX configured..\n", __func__));
> + DEBUG ((DEBUG_ERROR, "%a: REST EX is not configured..\n",
> __func__));
> DeleteRestEx = TRUE;
> goto EXIT_FREE_ALL;
> + } else {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: REST EX is configured..\n",
> __func__));
> }
>
> //
> @@ -1207,6 +1178,8 @@ RedfishServiceGetNetworkInterface (
> EFI_REDFISH_DISCOVER_NETWORK_INTERFACE
> *ThisNetworkInterface;
> EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
>
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> +
> if ((This == NULL) || (NetworkIntfInstances == NULL) ||
> (NumberOfNetworkIntfs == NULL) ||
> (ImageHandle == NULL))
> {
> @@ -1244,29 +1217,23 @@ RedfishServiceGetNetworkInterface (
> 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;
> + if (!EFI_ERROR (Status)) {
> + if (!ThisNetworkInterface->IsIpv6) {
> + IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4,
> &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
> + } else {
> + IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6,
> &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in
> IPv6 address information.
> }
>
> - ThisNetworkInterfaceIntn =
> (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *)GetNextNode
> (&mEfiRedfishDiscoverNetworkInterface, &ThisNetworkInterfaceIntn->Entry);
> - continue;
> + ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn-
> >SubnetPrefixLength;
> }
>
> + CopyMem ((VOID *)&ThisNetworkInterface->MacAddress,
> &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn-
> >HwAddressSize);
> ThisNetworkInterface->IsIpv6 = FALSE;
> if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) {
> ThisNetworkInterface->IsIpv6 = TRUE;
> }
>
> - CopyMem ((VOID *)&ThisNetworkInterface->MacAddress,
> &ThisNetworkInterfaceIntn->MacAddress, ThisNetworkInterfaceIntn-
> >HwAddressSize);
> - if (!ThisNetworkInterface->IsIpv6) {
> - IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4,
> &ThisNetworkInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information.
> - } else {
> - IP6_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v6,
> &ThisNetworkInterfaceIntn->SubnetAddr.v6); // IPv6 subnet information in
> IPv6 address information.
> - }
> -
> - ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn-
> >SubnetPrefixLength;
> - ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
> + ThisNetworkInterface->VlanId = ThisNetworkInterfaceIntn->VlanId;
> RestExInstance->NumberOfNetworkInterfaces++;
> if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface,
> &ThisNetworkInterfaceIntn->Entry)) {
> break;
> @@ -1378,9 +1345,16 @@ RedfishServiceAcquireService (
> //
> Status1 = NetworkInterfaceGetSubnetInfo
> (TargetNetworkInterfaceInternal, ImageHandle);
> if (EFI_ERROR (Status1)) {
> - DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n",
> __func__));
> - FreePool (Instance);
> - continue;
> + //
> + // Get subnet information could be failed for
> EFI_REDFISH_DISCOVER_HOST_INTERFACE case.
> + // We will configure network in AddAndSignalNewRedfishService. So
> don't skip this
> + // target network interface.
> + //
> + if ((Flags & EFI_REDFISH_DISCOVER_HOST_INTERFACE) == 0) {
> + DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n",
> __func__));
> + FreePool (Instance);
> + continue;
> + }
> }
>
> NewInstance = TRUE;
> @@ -1448,6 +1422,7 @@ RedfishServiceAbortAcquire (
> IN EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *TargetNetworkInterface
> OPTIONAL
> )
> {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> // This function is used to abort Redfish service discovery through SSDP
> // on the network interface. SSDP is optionally suppoted by
> EFI_REDFISH_DISCOVER_PROTOCOL,
> // we dont have implementation for SSDP now.
> @@ -1477,6 +1452,8 @@ RedfishServiceReleaseService (
> EFI_REDFISH_DISCOVERED_INSTANCE *ThisRedfishInstance;
> EFI_REDFISH_DISCOVERED_INTERNAL_LIST *DiscoveredRedfishInstance;
>
> + DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry.\n", __func__));
> +
> if (IsListEmpty (&mRedfishInstanceList)) {
> DEBUG ((DEBUG_ERROR, "%a:No any discovered Redfish service.\n",
> __func__));
> return EFI_NOT_FOUND;
> @@ -1723,22 +1700,13 @@ BuildupNetworkInterface (
> EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance;
> EFI_TPL OldTpl;
> BOOLEAN NewNetworkInterfaceInstalled;
> - UINT8 IpType;
> UINTN ListCount;
>
> ListCount = (sizeof (mRequiredProtocol) / sizeof
> (REDFISH_DISCOVER_REQUIRED_PROTOCOL));
> NewNetworkInterfaceInstalled = FALSE;
> Index = 0;
>
> - // 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 (FilterProtocol (mRequiredProtocol[Index].ProtocolType, IpType)) {
> - continue;
> - }
> -
> Status = gBS->OpenProtocol (
> // Already in list?
> ControllerHandle,
> @@ -2164,6 +2132,7 @@ RedfishDiscoverDriverBindingStart (
> IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
> )
> {
> + DEBUG ((DEBUG_MANAGEABILITY, "%a:Entry.\n", __func__));
> return BuildupNetworkInterface (This, ControllerHandle);
> }
>
> --
> 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 (#112190): https://edk2.groups.io/g/devel/message/112190
Mute This Topic: https://groups.io/mt/103014230/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.