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 F2EC574003D for ; Thu, 9 Nov 2023 15:12:41 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=EOtW2u0epgjPwWbsOd4EQYj6qzu5tzwD/a0clZt2x3c=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:CC:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1699542760; v=1; b=wR51k0IGLkxuVwWMkOuzBxKyovpdVJi6DvpIZ5uk7B4xdxMWhCGR4uQve+DY6lWUNhe4GZvN Yp6DpUVlNIaO7H0Xm7RZNdhuXOEW8u04W2jF6dS+Yhkpv0ahiQ6ypnn+fyYzulZRqjI7DwpjEej KDsonCC2uSpMYELUOinT2jbM= X-Received: by 127.0.0.2 with SMTP id HRLFYY7687511xUFL3sCc40X; Thu, 09 Nov 2023 07:12:40 -0800 X-Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by mx.groups.io with SMTP id smtpd.web11.124130.1699542759971769644 for ; Thu, 09 Nov 2023 07:12:40 -0800 X-Received: from pps.filterd (m0279873.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3A9DK6Zf007407; Thu, 9 Nov 2023 15:12:29 GMT X-Received: from nasanppmta04.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3u8u2ts4qj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 09 Nov 2023 15:12:28 +0000 X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA04.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 3A9FCR6C017275 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 9 Nov 2023 15:12:27 GMT X-Received: from [10.111.128.58] (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1118.39; Thu, 9 Nov 2023 07:12:25 -0800 Message-ID: <1371cdaf-9a6a-4e63-8265-2ad0d6e62fb2@quicinc.com> Date: Thu, 9 Nov 2023 15:12:23 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v3] RedfishPkg: RedfishDiscoverDxe: Fix issue if IPv4 installed after RestEx To: , CC: Abner Chang , Nickle Wang , References: <20231107120605.2035-1-igork@ami.com> From: "Leif Lindholm" In-Reply-To: <20231107120605.2035-1-igork@ami.com> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-ORIG-GUID: 7bFu1J8g-VIpnPFhW0aTuBAPsLNhs4im X-Proofpoint-GUID: 7bFu1J8g-VIpnPFhW0aTuBAPsLNhs4im 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,quic_llindhol@quicinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: fuTpkZd20qdFqmbUm44KN9crx7686176AA= Content-Language: en-GB Content-Type: text/plain; charset="UTF-8"; format=flowed 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=wR51k0IG; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=quicinc.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 On 2023-11-07 12:06, Igor Kulchytskyy via groups.io wrote: > Supported function of the driver changed to wait for all newtwork Typo: newtwork -> network > interface to be installed. > Filer out the network interfaces which are not supported by Filer -> Filter > Redfish Host Interface. These sound like two separate changes? > Cc: Abner Chang > Cc: Nickle Wang > Signed-off-by: Igor Kulchytskyy > --- > RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 165 ++++++++++++++= ------ > 1 file changed, 117 insertions(+), 48 deletions(-) >=20 > diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c b/Redfish= Pkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c > index 23da3b968f..85e47843e4 100644 > --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c > +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c > @@ -322,9 +322,16 @@ GetTargetNetworkInterfaceInternal ( > { > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *ThisNetworkInterfac= e; >=20 > + if (IsListEmpty (&mEfiRedfishDiscoverNetworkInterface)) { > + return NULL; > + } > + > ThisNetworkInterface =3D (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTE= RNAL *)GetFirstNode (&mEfiRedfishDiscoverNetworkInterface); > while (TRUE) { > - if (CompareMem ((VOID *)&ThisNetworkInterface->MacAddress, &TargetNe= tworkInterface->MacAddress, ThisNetworkInterface->HwAddressSize) =3D=3D 0) = { > + if ((CompareMem ((VOID *)&ThisNetworkInterface->MacAddress, &TargetN= etworkInterface->MacAddress, ThisNetworkInterface->HwAddressSize) =3D=3D 0)= && > + ((TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface->Netwo= rkProtocolType =3D=3D ProtocolTypeTcp6)) || > + (!TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface->Netw= orkProtocolType =3D=3D ProtocolTypeTcp4)))) > + { This could really benefit from some helper macros. e.g. if the test could look like if ((MAC_COMPARE (ThisNetworkInterface, TargetNetworkInterface) =3D=3D 0) &= & (VALID_TCP6 (TargetNetworkInterface, ThisNetworkInterface) || VALID_TCP4 (TargetNetworkInterface, ThisNetworkInterface))) { > return ThisNetworkInterface; > } >=20 > @@ -354,6 +361,10 @@ GetTargetNetworkInterfaceInternalByController ( > { > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *ThisNetworkInterfac= e; >=20 > + if (IsListEmpty (&mEfiRedfishDiscoverNetworkInterface)) { > + return NULL; > + } > + > ThisNetworkInterface =3D (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTE= RNAL *)GetFirstNode (&mEfiRedfishDiscoverNetworkInterface); > while (TRUE) { > if (ThisNetworkInterface->OpenDriverControllerHandle =3D=3D Control= lerHandle) { > @@ -476,6 +487,42 @@ CheckIsIpVersion6 ( > return FALSE; > } >=20 > +/** > + This function returns the IP type supported by the Host Interface. > + > + @retval 00h is Unknown > + 01h is Ipv4 > + 02h is Ipv6 > + > +**/ > +UINT8 If this is just a local helper function, we probably want it STATIC. > +GetHiIpProtocolType ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + REDFISH_OVER_IP_PROTOCOL_DATA *Data; > + REDFISH_INTERFACE_DATA *DeviceDescriptor; > + > + Data =3D NULL; > + DeviceDescriptor =3D NULL; > + if (mSmbios =3D=3D NULL) { > + Status =3D gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL, (VOID= **)&mSmbios); > + if (EFI_ERROR (Status)) { > + return REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN; > + } > + } > + > + Status =3D RedfishGetHostInterfaceProtocolData (mSmbios, &DeviceDescri= ptor, &Data); // Search for SMBIOS type 42h > + if (!EFI_ERROR (Status) && (Data !=3D NULL) && > + (Data->HostIpAssignmentType =3D=3D RedfishHostIpAssignmentStatic)) > + { > + return Data->HostIpAddressFormat; > + } > + > + return REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN; > +} > + > /** > This function discover Redfish service through SMBIOS host interface. >=20 > @@ -512,6 +559,18 @@ DiscoverRedfishHostInterface ( >=20 > Status =3D RedfishGetHostInterfaceProtocolData (mSmbios, &DeviceDescr= iptor, &Data); // Search for SMBIOS type 42h > if (!EFI_ERROR (Status) && (Data !=3D NULL) && (DeviceDescriptor !=3D= NULL)) { > + if ((Instance->NetworkInterface->NetworkProtocolType =3D=3D Protocol= TypeTcp4) && > + (Data->HostIpAddressFormat !=3D REDFISH_HOST_INTERFACE_HOST_IP_A= DDRESS_FORMAT_IP4)) // IPv4 case > + { > + DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv4, but Host Inte= rface requires Ipv6\n", __func__)); > + return EFI_UNSUPPORTED; > + } else if ((Instance->NetworkInterface->NetworkProtocolType =3D=3D P= rotocolTypeTcp6) && > + (Data->HostIpAddressFormat !=3D REDFISH_HOST_INTERFACE_HO= ST_IP_ADDRESS_FORMAT_IP6)) // IPv6 case > + { > + DEBUG ((DEBUG_ERROR, "%a: Network Interface is IPv6, but Host Inte= rface requires IPv4\n", __func__)); > + return EFI_UNSUPPORTED; > + } > + > // > // Check if we can reach out Redfish service using this network int= erface. > // Check with MAC address using Device Descriptor Data Device Type = 04 and Type 05. > @@ -1102,6 +1161,7 @@ RedfishServiceGetNetworkInterface ( > OUT EFI_REDFISH_DISCOVER_NETWORK_INTERFACE **NetworkIntfInstances > ) > { > + EFI_STATUS Status; > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_INTERNAL *ThisNetworkInterfac= eIntn; > EFI_REDFISH_DISCOVER_NETWORK_INTERFACE *ThisNetworkInterfac= e; > EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance; > @@ -1141,13 +1201,23 @@ RedfishServiceGetNetworkInterface ( >=20 > ThisNetworkInterfaceIntn =3D (EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_= INTERNAL *)GetFirstNode (&mEfiRedfishDiscoverNetworkInterface); > while (TRUE) { > + // If Get Subnet Info failed then skip this interface > + Status =3D NetworkInterfaceGetSubnetInfo (ThisNetworkInterfaceIntn, = ImageHandle); // Get subnet info > + if (EFI_ERROR (Status)) { > + if (IsNodeAtEnd (&mEfiRedfishDiscoverNetworkInterface, &ThisNetwor= kInterfaceIntn->Entry)) { > + break; > + } > + > + ThisNetworkInterfaceIntn =3D (EFI_REDFISH_DISCOVER_NETWORK_INTERFA= CE_INTERNAL *)GetNextNode (&mEfiRedfishDiscoverNetworkInterface, &ThisNetwo= rkInterfaceIntn->Entry); > + continue; > + } > + > ThisNetworkInterface->IsIpv6 =3D FALSE; > if (CheckIsIpVersion6 (ThisNetworkInterfaceIntn)) { > ThisNetworkInterface->IsIpv6 =3D TRUE; > } >=20 > CopyMem ((VOID *)&ThisNetworkInterface->MacAddress, &ThisNetworkInt= erfaceIntn->MacAddress, ThisNetworkInterfaceIntn->HwAddressSize); > - NetworkInterfaceGetSubnetInfo (ThisNetworkInterfaceIntn, ImageHandle= ); // Get subnet info. > if (!ThisNetworkInterface->IsIpv6) { > IP4_COPY_ADDRESS (&ThisNetworkInterface->SubnetId.v4, &ThisNetwor= kInterfaceIntn->SubnetAddr.v4); // IPv4 subnet information. > } else { > @@ -1230,7 +1300,12 @@ RedfishServiceAcquireService ( >=20 > if (TargetNetworkInterface !=3D NULL) { > TargetNetworkInterfaceInternal =3D GetTargetNetworkInterfaceInterna= l (TargetNetworkInterface); > - NumNetworkInterfaces =3D 1; > + if (TargetNetworkInterfaceInternal =3D=3D NULL) { > + DEBUG ((DEBUG_ERROR, "%a:No network interface on platform.\n", __f= unc__)); > + return EFI_UNSUPPORTED; > + } > + > + NumNetworkInterfaces =3D 1; > } else { > TargetNetworkInterfaceInternal =3D (EFI_REDFISH_DISCOVER_NETWORK_IN= TERFACE_INTERNAL *)GetFirstNode (&mEfiRedfishDiscoverNetworkInterface); > NumNetworkInterfaces =3D NumberOfNetworkInterface (); > @@ -1260,7 +1335,13 @@ RedfishServiceAcquireService ( > // Get subnet information in case subnet information is not set b= ecause > // RedfishServiceGetNetworkInterfaces hasn't been called yet. > // > - NetworkInterfaceGetSubnetInfo (TargetNetworkInterfaceInternal, Ima= geHandle); > + Status1 =3D NetworkInterfaceGetSubnetInfo (TargetNetworkInterfaceI= nternal, ImageHandle); > + if (EFI_ERROR (Status1)) { > + DEBUG ((DEBUG_ERROR, "%a: Get subnet information fail.\n", __fun= c__)); > + FreePool (Instance); > + continue; > + } > + > NewInstance =3D TRUE; > } >=20 > @@ -1547,25 +1628,26 @@ TestForRequiredProtocols ( > ControllerHandle, > EFI_OPEN_PROTOCOL_TEST_PROTOCOL > ); > + if (EFI_ERROR (Status)) { > + return EFI_UNSUPPORTED; > + } > + > + Status =3D gBS->OpenProtocol ( > + ControllerHandle, > + gRequiredProtocol[Index].DiscoveredProtocolGuid, > + (VOID **)&Id, > + This->DriverBindingHandle, > + ControllerHandle, > + EFI_OPEN_PROTOCOL_GET_PROTOCOL > + ); > if (!EFI_ERROR (Status)) { > - Status =3D gBS->OpenProtocol ( > - ControllerHandle, > - gRequiredProtocol[Index].DiscoveredProtocolGuid, > - (VOID **)&Id, > - This->DriverBindingHandle, > - ControllerHandle, > - EFI_OPEN_PROTOCOL_GET_PROTOCOL > - ); > - if (EFI_ERROR (Status)) { > - if (Index =3D=3D ListCount - 1) { > - DEBUG ((DEBUG_INFO, "%a: all required protocols are found on t= his controller handle: %p.\n", __func__, ControllerHandle)); > - return EFI_SUCCESS; > - } > - } > + // Already installed > + return EFI_UNSUPPORTED; > } > } >=20 > - return EFI_UNSUPPORTED; > + DEBUG ((DEBUG_MANAGEABILITY, "%a: all required protocols are found on = this controller handle: %p.\n", __func__, ControllerHandle)); > + return EFI_SUCCESS; > } >=20 > /** > @@ -1600,10 +1682,24 @@ BuildupNetworkInterface ( > EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL *RestExInstance; > EFI_TPL OldTpl; > BOOLEAN NewNetworkInterfaceI= nstalled; > + UINT8 IpType; > + UINTN ListCount; >=20 > + ListCount =3D (sizeof (gRequiredProtocol) / sizeof = (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); > NewNetworkInterfaceInstalled =3D FALSE; > Index =3D 0; > - do { > + > + // Get IP Type to filter out unnecessary network protocol if possible > + IpType =3D GetHiIpProtocolType (); > + > + for (Index =3D 0; Index < ListCount; Index++) { > + // Check IP Type and skip an unnecessary network protocol if does no= t match > + if (((gRequiredProtocol[Index].ProtocolType =3D=3D ProtocolTypeTcp4)= && (IpType =3D=3D REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)) || > + ((gRequiredProtocol[Index].ProtocolType =3D=3D ProtocolTypeTcp6)= && (IpType =3D=3D REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4))) Similarly, this becomes a bit of a handful. But it also feels like the logic is incorrect? I.e. it won't filter out entries where IpType =3D=3D=20 REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN, or invalid? Shouldn't the test be ((gRequiredProtocol[Index].ProtocolType =3D=3D ProtocolTypeTcp4) && (IpType !=3D REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4)) and ((gRequiredProtocol[Index].ProtocolType =3D=3D ProtocolTypeTcp6) && (IpType !=3D REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6))) Either way, those tests could probably also do with being turned into macros to improve readability. / Leif > + { > + continue; > + } > + > Status =3D gBS->OpenProtocol ( > // Already in list? > ControllerHandle, > @@ -1614,11 +1710,6 @@ BuildupNetworkInterface ( > EFI_OPEN_PROTOCOL_GET_PROTOCOL > ); > if (!EFI_ERROR (Status)) { > - Index++; > - if (Index =3D=3D (sizeof (gRequiredProtocol) / sizeof (REDFISH_DIS= COVER_REQUIRED_PROTOCOL))) { > - break; > - } > - > continue; > } >=20 > @@ -1631,11 +1722,6 @@ BuildupNetworkInterface ( > EFI_OPEN_PROTOCOL_GET_PROTOCOL > ); > if (EFI_ERROR (Status)) { > - Index++; > - if (Index =3D=3D (sizeof (gRequiredProtocol) / sizeof (REDFISH_DIS= COVER_REQUIRED_PROTOCOL))) { > - break; > - } > - > continue; > } >=20 > @@ -1694,11 +1780,6 @@ BuildupNetworkInterface ( > ProtocolDiscoverIdPtr > ); > if (EFI_ERROR (Status)) { > - Index++; > - if (Index =3D=3D (sizeof (gRequiredProtocol) / sizeof (REDFISH_DIS= COVER_REQUIRED_PROTOCOL))) { > - break; > - } > - > continue; > } >=20 > @@ -1755,25 +1836,13 @@ BuildupNetworkInterface ( > } > } else { > DEBUG ((DEBUG_MANAGEABILITY, "%a: Not REST EX, continue with = next\n", __func__)); > - Index++; > - if (Index =3D=3D (sizeof (gRequiredProtocol) / sizeof (REDFISH= _DISCOVER_REQUIRED_PROTOCOL))) { > - break; > - } > - > continue; > } > } >=20 > return Status; > - } else { > - Index++; > - if (Index =3D=3D (sizeof (gRequiredProtocol) / sizeof (REDFISH_DIS= COVER_REQUIRED_PROTOCOL))) { > - break; > - } > - > - continue; > } > - } while (Index < (sizeof (gRequiredProtocol) / sizeof (REDFISH_DISCOVE= R_REQUIRED_PROTOCOL))); > + } >=20 > return EFI_DEVICE_ERROR; > } > -- > 2.37.1.windows.1 > -The information contained in this message may be confidential and propri= etary to American Megatrends (AMI). This communication is intended to be re= ad only by the individual or entity to whom it is addressed or by their des= ignee. 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 telepho= ne at 770-246-8600, and then delete or destroy all copies of the transmissi= on. >=20 >=20 >=20 >=20 >=20 -=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 (#110990): https://edk2.groups.io/g/devel/message/110990 Mute This Topic: https://groups.io/mt/102441003/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-