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>
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111656): https://edk2.groups.io/g/devel/message/111656
Mute This Topic: https://groups.io/mt/102763117/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-Hi Abner,
please find my comments below.
On Thu, Nov 23, 2023 at 9:47 AM <abner.chang@amd.com> wrote:
>
> 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>
> 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));
There is no format for __func__ argument.
> 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);
Not related to this patch, but while you are changing this could you
fix memory allocation issue in IdentifyUsbNicBmcChannel()?
The problem string is:
BmcUsbNic->MacAddress = AllocateZeroPool (sizeof
(BmcUsbNic->MacAddressSize));
Obviously sizeof() must be removed.
Also, AllocateZeroPool is not required because of the following
CopyMem (BmcUsbNic->MacAddress, &Snp->Mode->CurrentAddress,
BmcUsbNic->MacAddressSize);
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111706): https://edk2.groups.io/g/devel/message/111706
Mute This Topic: https://groups.io/mt/102763117/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
[AMD Official Use Only - General]
Mike, your comments will be fixed in V3 9/9.
Thanks for catching this.
Abner
> -----Original Message-----
> From: Mike Maslenkin <mike.maslenkin@gmail.com>
> Sent: Monday, November 27, 2023 3:38 AM
> 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 V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC
> searching algorithm
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> please find my comments below.
>
> On Thu, Nov 23, 2023 at 9:47 AM <abner.chang@amd.com> wrote:
> >
> > 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>
> > 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/PlatformHostInter
> faceBmcUsbNicLib.c
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> > index 95900579118..e5bf70cfd58 100644
> > ---
> a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.c
> > +++
> b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostInter
> faceBmcUsbNicLib.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));
>
> There is no format for __func__ argument.
>
>
> > 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);
>
> Not related to this patch, but while you are changing this could you
> fix memory allocation issue in IdentifyUsbNicBmcChannel()?
> The problem string is:
> BmcUsbNic->MacAddress = AllocateZeroPool (sizeof
> (BmcUsbNic->MacAddressSize));
> Obviously sizeof() must be removed.
> Also, AllocateZeroPool is not required because of the following
> CopyMem (BmcUsbNic->MacAddress, &Snp->Mode->CurrentAddress,
> BmcUsbNic->MacAddressSize);
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111713): https://edk2.groups.io/g/devel/message/111713
Mute This Topic: https://groups.io/mt/102763117/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.