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 3FABCAC0FD5 for ; Wed, 15 Nov 2023 10:59:36 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=bMcetwa8QGiuzf0HTLIGb7bQ4oA13mI8tgl5QMcYrJk=; 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=1700045974; v=1; b=gWMlWK+6vl3LzCTX9d+1ZjGQjdI10j34uEwapImErLAOd4/LwTxckrDmmav5CP8efZoxQsOh +3At1cCEXW59NhZuE+F12b5wdscPq5XiCienX4PLBC1sQcGKcRmr0r1tPoeGDxGtlaqpK9Jg2tv Cqp2idXFte5midsXTvWWMAXk= X-Received: by 127.0.0.2 with SMTP id ZcwEYY7687511xe0Uz28Acen; Wed, 15 Nov 2023 02:59:34 -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.10597.1700045974100430667 for ; Wed, 15 Nov 2023 02:59:34 -0800 X-Received: from pps.filterd (m0279869.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 3AF8Tm9u003226; Wed, 15 Nov 2023 10:59:21 GMT X-Received: from nasanppmta03.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3uck9016d5-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 15 Nov 2023 10:59:20 +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 3AFAxJpR008743 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 15 Nov 2023 10:59:19 GMT X-Received: from [10.251.43.200] (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; Wed, 15 Nov 2023 02:59:17 -0800 Message-ID: Date: Wed, 15 Nov 2023 10:59:15 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: edk2-stable202311 Code freeze process violation Re: [edk2-devel] [PATCH v5 2/2] RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish Discover flow To: , , Mike Maslenkin , "igork@ami.com" , "Gao, Liming" , "Kinney, Michael D" CC: Nickle Wang References: <20231114142815.1604-1-igork@ami.com> <3e8c08dc-a665-41f1-8c51-d4e2743bdac1@quicinc.com> From: "Leif Lindholm" In-Reply-To: X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-GUID: tx0B94N-nev7og4FkR7Ao-sQt0oWKPGd X-Proofpoint-ORIG-GUID: tx0B94N-nev7og4FkR7Ao-sQt0oWKPGd X-MIME-Autoconverted: from 8bit to quoted-printable by mx0a-0031df01.pphosted.com id 3AF8Tm9u003226 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: 6pvJnN2UhvciZyswuzzYB1ocx7686176AA= 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=gWMlWK+6; 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-15 03:55, Chang, Abner via groups.io wrote: > [AMD Official Use Only - General] >=20 > Just let you know I just merged this change. Igor can help to follow up t= he suggestions given by Leif and Mike. I was under the impression merging was disabled for everyone except Mike=20 and Liming during code freeze specifically to avoid this situation. Apparently, that isn't working. Regardless, this is a violation of the stable tag process. Liming: can you please revert these commits? Regards, Leif > Thanks > Abner >=20 >> -----Original Message----- >> From: Chang, Abner >> Sent: Wednesday, November 15, 2023 9:20 AM >> To: Mike Maslenkin ; devel@edk2.groups.io; >> igork@ami.com >> Cc: Leif Lindholm ; Nickle Wang >> >> Subject: RE: [edk2-devel] [PATCH v5 2/2] RedfishPkg: RedfishDiscoverDxe: >> Optimize the Redfish Discover flow >> >> Hi Mike and Leif, >> Thanks for your comments on this change. As we are rushing to get this >> change to be pulled in stable release 202312 this week, I will just merg= e this >> code to master branch and let the discussing keeps going. >> I think there is no functionality difference base on your suggestions, b= ut it's >> about the coding practice and readability. >> >> Hi Igor, >> Could you please resend the V6 after stable tag is released if Mike and = Leif's >> comment is reasonable to you? >> >> Thanks >> Abner >> >>> -----Original Message----- >>> From: Mike Maslenkin >>> Sent: Wednesday, November 15, 2023 7:53 AM >>> To: devel@edk2.groups.io; igork@ami.com >>> Cc: Leif Lindholm ; Chang, Abner >>> ; Nickle Wang >>> Subject: Re: [edk2-devel] [PATCH v5 2/2] RedfishPkg: RedfishDiscoverDxe= : >>> Optimize the Redfish Discover flow >>> >>> Caution: This message originated from an External Source. Use proper >> caution >>> when opening attachments, clicking links, or responding. >>> >>> >>> On Tue, Nov 14, 2023 at 9:57=E2=80=AFPM Igor Kulchytskyy via groups.io >>> wrote: >>>> >>>> Hi Leif, >>>> Please see my comments below. >>>> Thank you, >>>> Igor >>>> >>>> >>>> -----Original Message----- >>>> From: Leif Lindholm >>>> Sent: Tuesday, November 14, 2023 12:26 PM >>>> To: devel@edk2.groups.io; Igor Kulchytskyy >>>> Cc: Abner Chang ; Nickle Wang >>> >>>> Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v5 2/2] RedfishPkg: >>> RedfishDiscoverDxe: Optimize the Redfish Discover flow >>>> >>>> >>>> **CAUTION: The e-mail below is from an external source. Please exercis= e >>> caution before opening attachments, clicking links, or following guidan= ce.** >>>> >>>> On 2023-11-14 14:28, Igor Kulchytskyy via groups.io wrote: >>>>> Filter out the network interfaces which are not supported by >>>>> Redfish Host Interface. >>>>> >>>>> Cc: Abner Chang >>>>> Cc: Nickle Wang >>>>> Signed-off-by: Igor Kulchytskyy >>>>> --- >>>>> RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c | 163 >>> ++++++++++++++------ >>>>> RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h | 6 + >>>>> 2 files changed, 120 insertions(+), 49 deletions(-) >>>>> >>>>> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c >>> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c >>>>> index 0f622e05a9..ae83cd3c97 100644 >>>>> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c >>>>> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c >>>> >>>> >>>>> @@ -1601,10 +1681,22 @@ BuildupNetworkInterface ( >>>>> EFI_REDFISH_DISCOVER_REST_EX_INSTANCE_INTERNAL >>> *RestExInstance; >>>>> EFI_TPL OldTpl; >>>>> BOOLEAN NewNetworkInter= faceInstalled; >>>>> + UINT8 IpType; >>>>> + UINTN ListCount; >>>>> >>>>> + ListCount =3D (sizeof (gRequiredProtocol) / siz= eof >>> (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); >>>>> NewNetworkInterfaceInstalled =3D FALSE; >>>>> Index =3D 0; >>>>> - do { >>>>> + >>>>> + // Get IP Type to filter out unnecessary network protocol if possi= ble >>>>> + IpType =3D GetHiIpProtocolType (); >>>>> + >>>>> + for (Index =3D 0; Index < ListCount; Index++) { >>>>> + // Check IP Type and skip an unnecessary network protocol if doe= s >> not >>> match >>>>> + if (IS_TCP4_MATCH (IpType) || IS_TCP6_MATCH (IpType)) { >>>> >>>> The logic of these macros is inverted compared to their names, though. >>>> >>>> You want this test to read >>>> if (!IS_TCP4_MATCH (IpType) && !IS_TCP6_MATCH (IpType)) { >>>> >>>>> + continue; >>>>> + } >>>>> + >>>>> Status =3D gBS->OpenProtocol ( >>>>> // Already in list? >>>>> ControllerHandle, >>>> >>>>> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h >>> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h >>>>> index 01454acc1d..3093eea0d5 100644 >>>>> --- a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h >>>>> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h >>>>> @@ -39,6 +39,12 @@ >>>>> #define REDFISH_DISCOVER_VERSION 0x00010000 >>>>> #define EFI_REDFISH_DISCOVER_NETWORK_INTERFACE_TPL >> TPL_NOTIFY >>>>> >>>>> +#define MAC_COMPARE(ThisNetworkInterface, >> TargetNetworkInterface) >>> (CompareMem ((VOID *)&ThisNetworkInterface->MacAddress, >>> &TargetNetworkInterface->MacAddress, ThisNetworkInterface- >>>> HwAddressSize)) >>>>> +#define VALID_TCP6(TargetNetworkInterface, ThisNetworkInterface) >>> (TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface- >>>> NetworkProtocolType =3D=3D ProtocolTypeTcp6)) >>>>> +#define VALID_TCP4(TargetNetworkInterface, ThisNetworkInterface) >>> (!TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface- >>>> NetworkProtocolType =3D=3D ProtocolTypeTcp4)) >>>>> +#define IS_TCP4_MATCH(IpType) >>> ((gRequiredProtocol[Index].ProtocolType =3D=3D ProtocolTypeTcp4) && >> (IpType !=3D >>> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4)) >>>>> +#define IS_TCP6_MATCH(IpType) >>> ((gRequiredProtocol[Index].ProtocolType =3D=3D ProtocolTypeTcp6) && >> (IpType !=3D >>> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)) >>>> >>>> And these macros to test for =3D=3D, not !=3D >>>> >>>> >>>> Igor: First version tested "=3D=3D", but we agreed that it may not wor= k if we >> have >>> a wrong value of IpType. >>>> >>>> Otherwise the code reads like it does the opposite of what it does. >>>> >>>> (You could also keep the logic and call the macros IS_TCP#_MISMATCH, b= ut >>>> that feels a bit convoluted.) >>>> >>>> Igor: I would prefer to go with IS_TCP#_MISMATCH names. >>>> >>>> Regards, >>>> >>>> Leif >>> >>> Sorry, could I add my 2 cents? >>> >>> For me all newly added defines looks bad, just because those >>> implicitly use reference to a global variable >>> plus local variable state (i.e current cycle index). >>> >>> Could we rewrite code in a simple and straight forward manner, similar = to: >>> >>> if (IpType =3D=3D >>> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN) { >>> // The protocol type is not specified in SMBIOS table type 42h >>> return EFI_UNSUPPORTED; >>> } >>> >>> for (Index =3D 0; Index < ListCount; Index++) { >>> if ((gRequiredProtocol[Index].ProtocolType =3D=3D ProtocolTypeTcp4) = && >>> (IpType !=3D >> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4)) { >>> continue; >>> } >>> if ((gRequiredProtocol[Index].ProtocolType =3D=3D ProtocolTypeTcp6) = && >>> (IpType !=3D >> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)) { >>> continue; >>> } >>> >>> >>> Regards, >>> Mike. >=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 (#111254): https://edk2.groups.io/g/devel/message/111254 Mute This Topic: https://groups.io/mt/102602698/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-