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 E8F5EAC1577 for ; Thu, 16 Nov 2023 12:25:58 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=8i7lQr4kbnEwh3tF+Kh3gEfTHvFHX0cktUsgTbaPD/w=; 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=1700137557; v=1; b=JuwJJksJkr3EknH/RLsM+RSGvRBgMdvMuHLZS0KBAGQiFw2zMHyx/us9DWmMLzjgyXKSoAYs 4vN5NkJoQmI8lhJ7zTP9pcxiBY4sSQqhZVa6LIzdly+w3WAE63sjNiHtkOpskVMTcF30Qo3SFlx D9RyWN+4upbG93rwF50egYFo= X-Received: by 127.0.0.2 with SMTP id CYqIYY7687511xdSQH5soCkt; Thu, 16 Nov 2023 04:25:57 -0800 X-Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by mx.groups.io with SMTP id smtpd.web10.5444.1700137556823351869 for ; Thu, 16 Nov 2023 04:25:57 -0800 X-Received: from pps.filterd (m0279868.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3AGCOMg4009797; Thu, 16 Nov 2023 12:25:53 GMT X-Received: from nasanppmta03.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3udg6f0buy-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 16 Nov 2023 12:25:52 +0000 X-Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA03.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 3AGCPqAY015135 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 16 Nov 2023 12:25:52 GMT X-Received: from [10.111.130.96] (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, 16 Nov 2023 04:25:50 -0800 Message-ID: <564f14bc-778c-4821-967b-7708030f57a3@quicinc.com> Date: Thu, 16 Nov 2023 12:25:47 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v5 2/2] RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish Discover flow To: Igor Kulchytskyy , "devel@edk2.groups.io" , "mike.maslenkin@gmail.com" CC: Abner Chang , Nickle Wang References: <20231114142815.1604-1-igork@ami.com> <3e8c08dc-a665-41f1-8c51-d4e2743bdac1@quicinc.com> <2f8fbb9f-bca8-4771-9178-6f0507ad089e@quicinc.com> <3ab53d06-fdcb-4a7b-a96d-4568e7219182@quicinc.com> From: "Leif Lindholm" In-Reply-To: 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: CwEUXUPq5u5uk4JNAM25di2sw-2L68-y X-Proofpoint-GUID: CwEUXUPq5u5uk4JNAM25di2sw-2L68-y X-MIME-Autoconverted: from 8bit to quoted-printable by mx0a-0031df01.pphosted.com id 3AGCOMg4009797 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: dkesBohIS0uNiIMF905JaCqhx7686176AA= 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=JuwJJksJ; 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 Ah, I fell off cc. Having a look now. / Leif On 2023-11-16 12:23, Igor Kulchytskyy wrote: > Hi Leif, > Already sent it yesterday. > Thank you, > Igor >=20 > -----Original Message----- > From: Leif Lindholm > Sent: Thursday, November 16, 2023 7:15 AM > To: devel@edk2.groups.io; mike.maslenkin@gmail.com; Igor Kulchytskyy > Cc: Abner Chang ; Nickle Wang > Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v5 2/2] RedfishPkg: RedfishDi= scoverDxe: Optimize the Redfish Discover flow >=20 >=20 > **CAUTION: The e-mail below is from an external source. Please exercise c= aution before opening attachments, clicking links, or following guidance.** >=20 > On 2023-11-15 18:27, Mike Maslenkin wrote: >> On Wed, Nov 15, 2023 at 4:52=E2=80=AFPM Igor Kulchytskyy = wrote: >>> >>> Hello Leif and Mike, >>> Let me try to explain the idea of the filtering IP. >>> That filtering should work only if we know exactly that IP is IPv4 or I= Pv6 in SMBIOS Type 42. >> Hm. I've already composed a reply below, but then a returned to this >> statement... >> >> Is this a difference in condition between v3 and v5? I came to the >> conclusion that at the place we are discussing >> SMBIOS table 42h can be absent because >> PlatformHostInterfaceInformationReady hasn't been called yet, >> so REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN is expected. >> >> >>> And it just helping to reduce the work in case we know the exact type o= f IP, which supposed to be used in BIOS BMC communication. >>> In that case there is no need to build network interface for the unused= IP Type. >>> On some systems IP address could be dynamic and we will not be able to = know the version of IP from SMBIOS. >>> If you see I check HostIpAssignmentType in GetHiIpProtocolType function= . And return IP type UNKNOWN if it is not static. >>> If we get an unknown IP type, we should not return from BuildupNetworkI= nterface function, but just proceed and build the network interface for all= IPs. >>> So, later Redfish Discover driver can find the right one. >>> Based on this logic I'm going to prepare the patch v6. >>> Thank you, >>> Igor >> >> Agree.. I was focused on edk2 implementation of >> RedfishPlatformHostInterfaceLib and PlatformHostInterfaceBmcUsbNicLib >> where HostIpAddressFormat is specified (hardcoded). I guess >> HostIpAddressFormat as well as SMBIOS table 42h must be available >> by the time RedfishServiceAcquireService()->DiscoverRedfishHostInterface= () >> call, while it might be not available at the moment >> RedfishDiscoverDriverBindingStart()->BuildupNetworkInterface(). So, >> condition from v3 looks correct to me. >> >> My main concern was introduction of defines. Those don't look great. >> Those are huge (it even doesn't fit into the screen) and misleading a >> bit. >> For example: >> +#define MAC_COMPARE(ThisNetworkInterface, TargetNetworkInterface) >> (CompareMem ((VOID *)&ThisNetworkInterface->MacAddress, >> &TargetNetworkInterface->MacAddress, >> ThisNetworkInterface->HwAddressSize)) >> >> The proposed variant is equal to #define MAC_COMPARE(A, B) >> (CompareMem ((VOID *)&ThisNetworkInterface->MacAddress, >> &TargetNetworkInterface->MacAddress, >> ThisNetworkInterface->HwAddressSize)), i.e a bit useless. >> >> I would expect it could be declared at least as: >> #define MAC_COMPARE(This, Target) CompareMem ((VOID >> *)&(This)->MacAddress, &(Target)->MacAddress, (This)->HwAddressSize) >> I.e define should really replace some arguments also reducing the line = length. >> >> BTW: there is a place in ValidateTargetNetworkInterface() where >> CompareMem can be replaced with MAC_COMPARE too. >> >> Also, I found IP6_LINK_EQUAL(Mac1, Mac2) define, that is unused in >> edk2. But according to that one, please consider moving "=3D=3D 0" check >> to #define declaration. >> But I do not think this macro is required at all, because there are 5 >> MAC compares left in this module. So, it just brings some >> inconsistency. >> >> Agreed that static helper function would be the best. >> >> Leif, do you expect something like this? >> STATIC >> BOOLEAN >> FilterInterface ( >> IN NETWORK_INTERFACE_PROTOCOL_TYPE ProtocolType, >> IN UINT8 HostIpAddressFormat >> ) >> { >> // This is based on v5, but according to the comments above >> // v3 is correct as it allows >> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN >> >> if (ProtocolType =3D=3D ProtocolTypeTcp4) { >> return IpType !=3D REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_I= P4; >> } else if (ProtocolType =3D=3D ProtocolTypeTcp6) { >> return IpType !=3D REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_I= P6; >> } >=20 > Yes, this looks ideal. >=20 >> return false; >=20 > Although this should be FALSE (upper-case). >=20 >> } >> >> and then:: >> >> // 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 (FilterInterface (gRequiredProtocol[Index].ProtocolType, IpType)) = { >=20 > This gives us a big but still readable chunk here, and much neater tests > in the helper than they *could* be either in place or in macros. >=20 >> continue; >> } >=20 > Ship it. >=20 > / > Leif >=20 >> Regards, >> Mike. >> >> >>=20 >> >> >=20 > -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. -=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 (#111316): https://edk2.groups.io/g/devel/message/111316 Mute This Topic: https://groups.io/mt/102584140/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-