public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH V2] RedfishPkg/RedfishDicovery: Remedy Redfish service discovery flow
@ 2023-12-11 14:58 Chang, Abner via groups.io
  2023-12-12 21:31 ` Mike Maslenkin
  0 siblings, 1 reply; 3+ messages in thread
From: Chang, Abner via groups.io @ 2023-12-11 14:58 UTC (permalink / raw)
  To: devel; +Cc: Nickle Wang, Igor Kulchytskyy, Mike Maslenkin

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.

Change-Id: Ibb38ddcd17459ad4b23fcb4fcd8a771a0f63987a
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   | 109 +++++++-----------
 1 file changed, 39 insertions(+), 70 deletions(-)

diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
index 833ae2b969f..06d8d00da7f 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;
@@ -675,6 +641,9 @@ DiscoverRedfishHostInterface (
       IsHttps = FALSE;
       if (Data->RedfishServiceIpPort == 443) {
         IsHttps = TRUE;
+        DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 443\n"));
+      } else {
+        DEBUG ((DEBUG_MANAGEABILITY, "Redfish service port: 80\n"));
       }
 
       StrSize = sizeof (UuidStr);
@@ -737,8 +706,6 @@ DiscoverRedfishHostInterface (
                  IsHttps
                  );
     }
-  } else {
-    DEBUG ((DEBUG_ERROR, "%a: RedfishGetHostInterfaceProtocolData is failed.\n", __func__));
   }
 
   return Status;
@@ -1015,9 +982,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 +1176,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))
   {
@@ -1242,31 +1213,27 @@ 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);
-    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.
+    //
+    // If Get Subnet Info failed then skip this interface
+    //
+    Status = NetworkInterfaceGetSubnetInfo (ThisNetworkInterfaceIntn, ImageHandle); // Get subnet info
+    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.
+      }
+
+      ThisNetworkInterface->SubnetPrefixLength = ThisNetworkInterfaceIntn->SubnetPrefixLength;
     }
 
-    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 (#112303): https://edk2.groups.io/g/devel/message/112303
Mute This Topic: https://groups.io/mt/103109949/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] 3+ messages in thread
end of thread, other threads:[~2023-12-12 21:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <179FCEF958C95E57.24323@groups.io>
2023-12-12  0:24 ` [edk2-devel] [PATCH V2] RedfishPkg/RedfishDicovery: Remedy Redfish service discovery flow Chang, Abner via groups.io
2023-12-11 14:58 Chang, Abner via groups.io
2023-12-12 21:31 ` Mike Maslenkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox