Re: [edk2-devel] [PATCH V3 1/9] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm

["Igor Kulchytskyy via groups.io" <igork=ami.com@groups.io>]

Reviewed-by: Igor Kulchytskyy <igork@ami.com>

Regards,
Igor

-----Original Message-----
From: abner.chang@amd.com <abner.chang@amd.com>
Sent: Monday, November 27, 2023 12:31 AM
To: devel@edk2.groups.io
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>; Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
Subject: [EXTERNAL] [PATCH V3 1/9] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm


**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>

Update BMC USB NIC searching algorithm for IPv4 only.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Co-authored-by: Mike Maslenkin <mike.maslenkin@gmail.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
Cc: Mike Maslenkin <mike.maslenkin@gmail.com>
---
 .../PlatformHostInterfaceBmcUsbNicLib.c       | 188 ++++++++++++------
 1 file changed, 128 insertions(+), 60 deletions(-)

diff --git a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
index 95900579118..e5bf70cfd58 100644
--- a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
+++ b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInterfaceBmcUsbNicLib.c
@@ -368,7 +368,9 @@ RetrievedBmcUsbNicInfo (
         ));
       CopyMem ((VOID *)&ThisInstance->RedfishIpAddressIpv4, (VOID *)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
       //
-      // According to UEFI spec, the IP address at BMC USB NIC host end is the IP address at BMC end minus 1.
+      // According to the design spec:
+      // https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-bmc-and-the-bmc-exposed-usb-network-device
+      // The IP address at BMC USB NIC host end is the IP address at BMC end minus 1.
       //
       CopyMem ((VOID *)&ThisInstance->HostIpAddressIpv4, (VOID *)&DestIpAddress->IpAddress, sizeof (DestIpAddress->IpAddress));
       ThisInstance->HostIpAddressIpv4[sizeof (ThisInstance->HostIpAddressIpv4) - 1] -= 1;
@@ -729,8 +731,10 @@ HostInterfaceIpmiCheckMacAddress (

       //
       // According to design spec in Readme file under RedfishPkg.
-      // Compare the first five MAC address and
-      // the 6th MAC address.
+      // https://github.com/tianocore/edk2/tree/master/RedfishPkg#platform-with-bmc-and-the-bmc-exposed-usb-network-device
+      // Compare the first five elements of MAC address and the 6th element of MAC address.
+      // The 6th element of MAC address must be the 6th element of
+      // IPMI channel MAC address minus 1.
       //
       if ((IpmiLanMacAddressSize != UsbNicInfo->MacAddressSize) ||
           (CompareMem (
@@ -738,8 +742,8 @@ HostInterfaceIpmiCheckMacAddress (
              (VOID *)&IpmiLanChannelMacAddress.Addr,
              IpmiLanMacAddressSize - 1
              ) != 0) ||
-          (IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] !=
-           *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1) - 1)
+          ((IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] - 1) !=
+           *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1))
           )
       {
         DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address is not matched.\n"));
@@ -962,6 +966,49 @@ UsbNicSearchUsbIo (
   return EFI_NOT_FOUND;
 }

+/**
+  This function identifies if the USB NIC has MAC address and internet
+  protocol device path installed. (Only support IPv4)
+
+  @param[in] UsbDevicePath     USB device path.
+
+  @retval EFI_SUCCESS          Yes, this is IPv4 SNP handle
+  @retval EFI_NOT_FOUND        No, this is not IPv4 SNP handle
+
+**/
+EFI_STATUS
+IdentifyNetworkMessageDevicePath (
+  IN EFI_DEVICE_PATH_PROTOCOL  *UsbDevicePath
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+
+  DevicePath = UsbDevicePath;
+  while (TRUE) {
+    DevicePath = NextDevicePathNode (DevicePath);
+    if (IsDevicePathEnd (DevicePath)) {
+      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "MAC address device path is not found on this handle.\n"));
+      break;
+    }
+
+    if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_MAC_ADDR_DP)) {
+      DevicePath = NextDevicePathNode (DevicePath); // Advance to next device path protocol.
+      if (IsDevicePathEnd (DevicePath)) {
+        DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "IPv4 device path is not found on this handle.\n"));
+        break;
+      }
+
+      if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_IPv4_DP)) {
+        return EFI_SUCCESS;
+      }
+
+      break;
+    }
+  }
+
+  return EFI_NOT_FOUND;
+}
+
 /**
   This function identifies if the USB NIC is exposed by BMC as
   the host-BMC channel.
@@ -1025,7 +1072,7 @@ IdentifyUsbNicBmcChannel (
     (VOID *)&Snp->Mode->CurrentAddress,
     BmcUsbNic->MacAddressSize
     );
-  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address (in size %d) for this SNP instance:\n      ", BmcUsbNic->MacAddressSize));
+  DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    MAC address (in size %d) for this SNP instance:\n", BmcUsbNic->MacAddressSize));
   for (Index = 0; Index < BmcUsbNic->MacAddressSize; Index++) {
     DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "%02x ", *(BmcUsbNic->MacAddress + Index)));
   }
@@ -1068,7 +1115,8 @@ CheckBmcUsbNicOnHandles (
   UINTN                     Index;
   EFI_STATUS                Status;
   EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
-  BOOLEAN                   GotOneUsbNIc;
+  BOOLEAN                   GotBmcUsbNic;
+  CHAR16                    *DevicePathStr;

   if ((HandleNumer == 0) || (HandleBuffer == NULL)) {
     return EFI_INVALID_PARAMETER;
@@ -1076,26 +1124,37 @@ CheckBmcUsbNicOnHandles (

   DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry, #%d SNP handle\n", __func__, HandleNumer));

-  GotOneUsbNIc = FALSE;
+  GotBmcUsbNic = FALSE;
   for (Index = 0; Index < HandleNumer; Index++) {
+    DEBUG ((DEBUG_MANAGEABILITY, "    Locate device path on handle 0x%08x\n", *(HandleBuffer + Index)));
     Status = gBS->HandleProtocol (
                     *(HandleBuffer + Index),
                     &gEfiDevicePathProtocolGuid,
                     (VOID **)&DevicePath
                     );
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "    Failed to locate SNP on %d handle.\n", Index));
+      DEBUG ((DEBUG_ERROR, "    Failed to locate device path on %d handle.\n", __func__, Index));
       continue;
     }

+    DevicePathStr = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
+    if (DevicePathStr != NULL) {
+      DEBUG ((DEBUG_MANAGEABILITY, "    Device path: %s\n", DevicePathStr));
+      FreePool (DevicePathStr);
+    }
+
     // Check if this is an BMC exposed USB NIC device.
     while (TRUE) {
       if ((DevicePath->Type == MESSAGING_DEVICE_PATH) && (DevicePath->SubType == MSG_USB_DP)) {
-        Status = IdentifyUsbNicBmcChannel (*(HandleBuffer + Index), DevicePath);
+        Status = IdentifyNetworkMessageDevicePath (DevicePath);
         if (!EFI_ERROR (Status)) {
-          GotOneUsbNIc = TRUE;
-          break;
+          Status = IdentifyUsbNicBmcChannel (*(HandleBuffer + Index), DevicePath);
+          if (!EFI_ERROR (Status)) {
+            GotBmcUsbNic = TRUE;
+          }
         }
+
+        break; // Advance to next SNP handle.
       }

       DevicePath = NextDevicePathNode (DevicePath);
@@ -1105,10 +1164,11 @@ CheckBmcUsbNicOnHandles (
     }
   }

-  if (GotOneUsbNIc) {
+  if (GotBmcUsbNic) {
     return EFI_SUCCESS;
   }

+  DEBUG ((DEBUG_MANAGEABILITY, "No BMC USB NIC found on SNP handles\n"));
   return EFI_NOT_FOUND;
 }

@@ -1139,62 +1199,70 @@ CheckBmcUsbNic (

   DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry, the registration key - 0x%08x.\n", __func__, Registration));

-  Handle     = NULL;
-  Status     = EFI_SUCCESS;
-  BufferSize = 0;
+  Handle = NULL;
+  Status = EFI_SUCCESS;

-  Status = gBS->LocateHandle (
-                  Registration == NULL ? ByProtocol : ByRegisterNotify,
-                  &gEfiSimpleNetworkProtocolGuid,
-                  Registration,
-                  &BufferSize,
-                  NULL
-                  );
-  if (Status == EFI_BUFFER_TOO_SMALL) {
-    DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    %d SNP protocol instances.\n", BufferSize/sizeof (EFI_HANDLE)));
-    HandleBuffer = AllocateZeroPool (BufferSize);
-    if (HandleBuffer == NULL) {
-      DEBUG ((DEBUG_ERROR, "    Falied to allocate buffer for the handles.\n"));
-      return EFI_OUT_OF_RESOURCES;
-    }
+  do {
+    BufferSize = 0;
+    Status     = gBS->LocateHandle (
+                        Registration == NULL ? ByProtocol : ByRegisterNotify,
+                        &gEfiSimpleNetworkProtocolGuid,
+                        Registration,
+                        &BufferSize,
+                        NULL
+                        );
+    if (Status == EFI_BUFFER_TOO_SMALL) {
+      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    %d SNP protocol instance(s).\n", BufferSize/sizeof (EFI_HANDLE)));
+      HandleBuffer = AllocateZeroPool (BufferSize);
+      if (HandleBuffer == NULL) {
+        DEBUG ((DEBUG_ERROR, "    Falied to allocate buffer for the handles.\n"));
+        return EFI_OUT_OF_RESOURCES;
+      }
+
+      Status = gBS->LocateHandle (
+                      Registration == NULL ? ByProtocol : ByRegisterNotify,
+                      &gEfiSimpleNetworkProtocolGuid,
+                      Registration,
+                      &BufferSize,
+                      HandleBuffer
+                      );
+      if (EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_ERROR, "    Falied to locate SNP protocol handles.\n"));
+        FreePool (HandleBuffer);
+        return Status;
+      }
+    } else if (EFI_ERROR (Status)) {
+      if (Registration != NULL) {
+        DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    No more newly installed SNP protocol for this registration - %r.\n", Status));
+        return EFI_SUCCESS;
+      }

-    Status = gBS->LocateHandle (
-                    Registration == NULL ? ByProtocol : ByRegisterNotify,
-                    &gEfiSimpleNetworkProtocolGuid,
-                    Registration,
-                    &BufferSize,
-                    HandleBuffer
-                    );
-    if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "    Falied to locate SNP protocol handles.\n"));
-      FreePool (HandleBuffer);
       return Status;
     }
-  } else if (EFI_ERROR (Status)) {
-    return Status;
-  }

-  // Check USB NIC on handles.
-  Status = CheckBmcUsbNicOnHandles (BufferSize/sizeof (EFI_HANDLE), HandleBuffer);
-  if (!EFI_ERROR (Status)) {
-    // Retrieve the rest of BMC USB NIC information for Redfish over IP information
-    // and USB Network Interface V2.
-    Status = RetrievedBmcUsbNicInfo ();
+    // Check USB NIC on handles.
+    Status = CheckBmcUsbNicOnHandles (BufferSize/sizeof (EFI_HANDLE), HandleBuffer);
     if (!EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    Install protocol to notify the platform Redfish Host Interface information is ready.\n"));
-      Status = gBS->InstallProtocolInterface (
-                      &Handle,
-                      &mPlatformHostInterfaceBmcUsbNicReadinessGuid,
-                      EFI_NATIVE_INTERFACE,
-                      NULL
-                      );
-      if (EFI_ERROR (Status)) {
-        DEBUG ((DEBUG_ERROR, "    Install protocol fail %r.\n", Status));
+      // Retrieve the rest of BMC USB NIC information for Redfish over IP information
+      // and USB Network Interface V2.
+      Status = RetrievedBmcUsbNicInfo ();
+      if (!EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "    Install protocol to notify the platform Redfish Host Interface information is ready.\n"));
+        Status = gBS->InstallProtocolInterface (
+                        &Handle,
+                        &mPlatformHostInterfaceBmcUsbNicReadinessGuid,
+                        EFI_NATIVE_INTERFACE,
+                        NULL
+                        );
+        if (EFI_ERROR (Status)) {
+          DEBUG ((DEBUG_ERROR, "    Install protocol fail %r.\n", Status));
+        }
       }
     }
-  }

-  FreePool (HandleBuffer);
+    FreePool (HandleBuffer);
+  } while (Registration != NULL);
+
   return Status;
 }

--
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 (#111990): https://edk2.groups.io/g/devel/message/111990
Mute This Topic: https://groups.io/mt/102824318/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-