From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 2FF8C74003C for ; Sun, 26 Nov 2023 19:38:52 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=NVfxnue8CyBYuEkrBVos0PQqEjuyJB57Xy8lCdvxXmk=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1701027530; v=1; b=lVsUfyzdCNh09z/yNNT4SpXj7ZcNNmaPwYbJDrOsXKHx8PTJz2hiAhRg7wPVZMJfSsEto3aL vMw6HQ6qcwppMKZQMxy5QqiYy0amjwJeHD+XiPKWmowVudF0Pi6g3UZGuv/1NK+FgAfzy6BrxZL Ia9OQafTS9JA/e2ktxocf5dU= X-Received: by 127.0.0.2 with SMTP id 0HzeYY7687511xpr7NaEcz4D; Sun, 26 Nov 2023 11:38:50 -0800 X-Received: from mail-yb1-f178.google.com (mail-yb1-f178.google.com [209.85.219.178]) by mx.groups.io with SMTP id smtpd.web10.64436.1701027530202234674 for ; Sun, 26 Nov 2023 11:38:50 -0800 X-Received: by mail-yb1-f178.google.com with SMTP id 3f1490d57ef6-db4422fff15so2643606276.1 for ; Sun, 26 Nov 2023 11:38:50 -0800 (PST) X-Gm-Message-State: t9LPLqWe9MxisH9KOT8saBNhx7686176AA= X-Google-Smtp-Source: AGHT+IFSp6FVLv5AxOzNHDRNEEC4m70EaTOYIYEqxlUn4jWTwBJYfefgCAkGY3yuyYF74CZqlLIuaXoWrtQBevosxWk= X-Received: by 2002:a25:cfc9:0:b0:da0:37c8:9f00 with SMTP id f192-20020a25cfc9000000b00da037c89f00mr8890385ybg.36.1701027529376; Sun, 26 Nov 2023 11:38:49 -0800 (PST) MIME-Version: 1.0 References: <20231123064719.1248-1-abner.chang@amd.com> <20231123064719.1248-2-abner.chang@amd.com> In-Reply-To: <20231123064719.1248-2-abner.chang@amd.com> From: "Mike Maslenkin" Date: Sun, 26 Nov 2023 22:38:13 +0300 Message-ID: Subject: Re: [edk2-devel] [PATCH V2 1/8] RedfishPkg/BmcUsbNicLib: Update BMC USB NIC searching algorithm To: abner.chang@amd.com Cc: devel@edk2.groups.io, Nickle Wang , Igor Kulchytskyy Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mike.maslenkin@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=lVsUfyzd; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Hi Abner, please find my comments below. On Thu, Nov 23, 2023 at 9:47=E2=80=AFAM wrote: > > From: Abner Chang > > Update BMC USB NIC searching algorithm for IPv4 only. > > Signed-off-by: Abner Chang > Cc: Nickle Wang > Cc: Igor Kulchytskyy > Cc: Mike Maslenkin > --- > .../PlatformHostInterfaceBmcUsbNicLib.c | 188 ++++++++++++------ > 1 file changed, 128 insertions(+), 60 deletions(-) > > diff --git a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/Platfor= mHostInterfaceBmcUsbNicLib.c b/RedfishPkg/Library/PlatformHostInterfaceBmcU= sbNicLib/PlatformHostInterfaceBmcUsbNicLib.c > index 95900579118..e5bf70cfd58 100644 > --- a/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostIn= terfaceBmcUsbNicLib.c > +++ b/RedfishPkg/Library/PlatformHostInterfaceBmcUsbNicLib/PlatformHostIn= terfaceBmcUsbNicLib.c > @@ -368,7 +368,9 @@ RetrievedBmcUsbNicInfo ( > )); > CopyMem ((VOID *)&ThisInstance->RedfishIpAddressIpv4, (VOID *)&Des= tIpAddress->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#platfo= rm-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 *)&DestIp= Address->IpAddress, sizeof (DestIpAddress->IpAddress)); > ThisInstance->HostIpAddressIpv4[sizeof (ThisInstance->HostIpAddres= sIpv4) - 1] -=3D 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#platfo= rm-with-bmc-and-the-bmc-exposed-usb-network-device > + // Compare the first five elements of MAC address and the 6th elem= ent of MAC address. > + // The 6th element of MAC address must be the 6th element of > + // IPMI channel MAC address minus 1. > // > if ((IpmiLanMacAddressSize !=3D UsbNicInfo->MacAddressSize) || > (CompareMem ( > @@ -738,8 +742,8 @@ HostInterfaceIpmiCheckMacAddress ( > (VOID *)&IpmiLanChannelMacAddress.Addr, > IpmiLanMacAddressSize - 1 > ) !=3D 0) || > - (IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] !=3D > - *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1) - 1) > + ((IpmiLanChannelMacAddress.Addr[IpmiLanMacAddressSize - 1] - 1= ) !=3D > + *(UsbNicInfo->MacAddress + IpmiLanMacAddressSize - 1)) > ) > { > DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, " MAC address is not ma= tched.\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 =3D UsbDevicePath; > + while (TRUE) { > + DevicePath =3D 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 =3D=3D MESSAGING_DEVICE_PATH) && (DevicePath->= SubType =3D=3D MSG_MAC_ADDR_DP)) { > + DevicePath =3D NextDevicePathNode (DevicePath); // Advance to next= device path protocol. > + if (IsDevicePathEnd (DevicePath)) { > + DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "IPv4 device path is not f= ound on this handle.\n")); > + break; > + } > + > + if ((DevicePath->Type =3D=3D MESSAGING_DEVICE_PATH) && (DevicePath= ->SubType =3D=3D 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) fo= r this SNP instance:\n ", BmcUsbNic->MacAddressSize)); > + DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, " MAC address (in size %d) fo= r this SNP instance:\n", BmcUsbNic->MacAddressSize)); > for (Index =3D 0; Index < BmcUsbNic->MacAddressSize; Index++) { > DEBUG ((DEBUG_REDFISH_HOST_INTERFACE, "%02x ", *(BmcUsbNic->MacAddre= ss + Index))); > } > @@ -1068,7 +1115,8 @@ CheckBmcUsbNicOnHandles ( > UINTN Index; > EFI_STATUS Status; > EFI_DEVICE_PATH_PROTOCOL *DevicePath; > - BOOLEAN GotOneUsbNIc; > + BOOLEAN GotBmcUsbNic; > + CHAR16 *DevicePathStr; > > if ((HandleNumer =3D=3D 0) || (HandleBuffer =3D=3D NULL)) { > return EFI_INVALID_PARAMETER; > @@ -1076,26 +1124,37 @@ CheckBmcUsbNicOnHandles ( > > DEBUG ((DEBUG_MANAGEABILITY, "%a: Entry, #%d SNP handle\n", __func__, = HandleNumer)); > > - GotOneUsbNIc =3D FALSE; > + GotBmcUsbNic =3D FALSE; > for (Index =3D 0; Index < HandleNumer; Index++) { > + DEBUG ((DEBUG_MANAGEABILITY, " Locate device path on handle 0x%08= x\n", *(HandleBuffer + Index))); > Status =3D gBS->HandleProtocol ( > *(HandleBuffer + Index), > &gEfiDevicePathProtocolGuid, > (VOID **)&DevicePath > ); > if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, " Failed to locate SNP on %d handle.\n", I= ndex)); > + DEBUG ((DEBUG_ERROR, " Failed to locate device path on %d handl= e.\n", __func__, Index)); There is no format for __func__ argument. > continue; > } > > + DevicePathStr =3D ConvertDevicePathToText (DevicePath, FALSE, FALSE)= ; > + if (DevicePathStr !=3D NULL) { > + DEBUG ((DEBUG_MANAGEABILITY, " Device path: %s\n", DevicePathSt= r)); > + FreePool (DevicePathStr); > + } > + > // Check if this is an BMC exposed USB NIC device. > while (TRUE) { > if ((DevicePath->Type =3D=3D MESSAGING_DEVICE_PATH) && (DevicePath= ->SubType =3D=3D MSG_USB_DP)) { > - Status =3D IdentifyUsbNicBmcChannel (*(HandleBuffer + Index), De= vicePath); > + Status =3D IdentifyNetworkMessageDevicePath (DevicePath); > if (!EFI_ERROR (Status)) { > - GotOneUsbNIc =3D TRUE; > - break; > + Status =3D 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 =3D 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); -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-