Mike: Yes. We should set Read access in code freeze phase. Abner: In hard code freeze phase, the critical issue is still allowed to be merged after it got approval from Stewards. Can you resend the patch with edk2-stable202311 title to get maintainer review and Stewards approval? For the merged code, I agree Lefi to revert it first.
Leif: This is my mistake. I should set the correct access in the code freeze phase. For this patch, I suggest Abner goes through the critical patch review process if this patch needs to catch this stable tag. Thanks Liming > -----邮件原件----- > 发件人: Kinney, Michael D <michael.d.kin...@intel.com> > 发送时间: 2023年11月16日 1:29 > 收件人: Chang, Abner <abner.ch...@amd.com>; Leif Lindholm > <quic_llind...@quicinc.com>; devel@edk2.groups.io; Mike Maslenkin > <mike.maslen...@gmail.com>; ig...@ami.com; Gao, Liming > <gaolim...@byosoft.com.cn> > 抄送: Nickle Wang <nick...@nvidia.com>; Kinney, Michael D > <michael.d.kin...@intel.com> > 主题: RE: edk2-stable202311 Code freeze process violation Re: [edk2-devel] > [PATCH v5 2/2] RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish Discover > flow > > Hello, > > Looks like the process for permissions needs to be adjusted during soft/hard > freeze. > > Liming reduced EDK II Maintainers team to "Triage" for edk2 repo. > > But from this documentation lists Triage as allowing add/remove labels. > > https://docs.github.com/en/organizations/managing-user-access-to-your-org > anizations-repositories/managing-repository-roles/repository-roles-for-an-org > anization > > Looks like reducing to EDK II Maintainer team to "Read" is the right setting > for > soft/hard freeze. > > Mike > > > > -----Original Message----- > > From: Chang, Abner <abner.ch...@amd.com> > > Sent: Wednesday, November 15, 2023 3:55 AM > > To: Leif Lindholm <quic_llind...@quicinc.com>; devel@edk2.groups.io; > > Mike Maslenkin <mike.maslen...@gmail.com>; ig...@ami.com; Gao, > Liming > > <gaolim...@byosoft.com.cn>; Kinney, Michael D > > <michael.d.kin...@intel.com> > > Cc: Nickle Wang <nick...@nvidia.com> > > Subject: RE: edk2-stable202311 Code freeze process violation Re: > > [edk2-devel] [PATCH v5 2/2] RedfishPkg: RedfishDiscoverDxe: Optimize > > the Redfish Discover flow > > > > [AMD Official Use Only - General] > > > > Ok, Liming as we are going to revert that two commits. Is that > > possible to wait for Igor sending out another patch to address Leif's > > comment? This may delay the stable release a little bit. > > As without this patch, users may encounter the problem when sending > > request to Redfish service on the platform which have multiple NIC > > installed, > > > > Let us know, thanks! > > Abner > > > > > -----Original Message----- > > > From: Chang, Abner > > > Sent: Wednesday, November 15, 2023 7:07 PM > > > To: Leif Lindholm <quic_llind...@quicinc.com>; devel@edk2.groups.io; > > Mike > > > Maslenkin <mike.maslen...@gmail.com>; ig...@ami.com; Gao, Liming > > > <gaolim...@byosoft.com.cn>; Kinney, Michael D > > > <michael.d.kin...@intel.com> > > > Cc: Nickle Wang <nick...@nvidia.com> > > > Subject: RE: edk2-stable202311 Code freeze process violation Re: > > [edk2- > > > devel] [PATCH v5 2/2] RedfishPkg: RedfishDiscoverDxe: Optimize the > > Redfish > > > Discover flow > > > > > > Hi Leif, > > > As we requested Liming to wait for this change last week, he > > accepted to wait > > > for the PR. But you are right, suppose I shouldn't be allowed to > > merge the > > > change during code freeze. Maybe only certain people have privilege > > to merge > > > the code during code freeze. If I still can merge the code then the > > mechanism > > > may be broken. I am fine if you would like to revert these commits. > > > > > > Regards, > > > Abner > > > > > > > -----Original Message----- > > > > From: Leif Lindholm <quic_llind...@quicinc.com> > > > > Sent: Wednesday, November 15, 2023 6:59 PM > > > > To: devel@edk2.groups.io; Chang, Abner <abner.ch...@amd.com>; > Mike > > > > Maslenkin <mike.maslen...@gmail.com>; ig...@ami.com; Gao, Liming > > > > <gaolim...@byosoft.com.cn>; Kinney, Michael D > > > > <michael.d.kin...@intel.com> > > > > Cc: Nickle Wang <nick...@nvidia.com> > > > > Subject: edk2-stable202311 Code freeze process violation 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 2023-11-15 03:55, Chang, Abner via groups.io wrote: > > > > > [AMD Official Use Only - General] > > > > > > > > > > Just let you know I just merged this change. Igor can help to > > follow up the > > > > suggestions given by Leif and Mike. > > > > > > > > I was under the impression merging was disabled for everyone > > except Mike > > > > 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 > > > > > > > > > >> -----Original Message----- > > > > >> From: Chang, Abner > > > > >> Sent: Wednesday, November 15, 2023 9:20 AM > > > > >> To: Mike Maslenkin <mike.maslen...@gmail.com>; > > > devel@edk2.groups.io; > > > > >> ig...@ami.com > > > > >> Cc: Leif Lindholm <quic_llind...@quicinc.com>; Nickle Wang > > > > >> <nick...@nvidia.com> > > > > >> 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 merge > > > this > > > > >> code to master branch and let the discussing keeps going. > > > > >> I think there is no functionality difference base on your > > suggestions, but > > > 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 <mike.maslen...@gmail.com> > > > > >>> Sent: Wednesday, November 15, 2023 7:53 AM > > > > >>> To: devel@edk2.groups.io; ig...@ami.com > > > > >>> Cc: Leif Lindholm <quic_llind...@quicinc.com>; Chang, Abner > > > > >>> <abner.ch...@amd.com>; Nickle Wang <nick...@nvidia.com> > > > > >>> 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 PM Igor Kulchytskyy via groups.io > > > > >>> <igork=ami....@groups.io> wrote: > > > > >>>> > > > > >>>> Hi Leif, > > > > >>>> Please see my comments below. > > > > >>>> Thank you, > > > > >>>> Igor > > > > >>>> > > > > >>>> > > > > >>>> -----Original Message----- > > > > >>>> From: Leif Lindholm <quic_llind...@quicinc.com> > > > > >>>> Sent: Tuesday, November 14, 2023 12:26 PM > > > > >>>> To: devel@edk2.groups.io; Igor Kulchytskyy <ig...@ami.com> > > > > >>>> Cc: Abner Chang <abner.ch...@amd.com>; Nickle Wang > > > > >>> <nick...@nvidia.com> > > > > >>>> 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 > > > exercise > > > > >>> caution before opening attachments, clicking links, or > > following > > > > guidance.** > > > > >>>> > > > > >>>> 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 <abner.ch...@amd.com> > > > > >>>>> Cc: Nickle Wang <nick...@nvidia.com> > > > > >>>>> Signed-off-by: Igor Kulchytskyy <ig...@ami.com> > > > > >>>>> --- > > > > >>>>> 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 > > NewNetworkInterfaceInstalled; > > > > >>>>> + UINT8 > IpType; > > > > >>>>> + UINTN > > ListCount; > > > > >>>>> > > > > >>>>> + ListCount = (sizeof > > (gRequiredProtocol) / sizeof > > > > >>> (REDFISH_DISCOVER_REQUIRED_PROTOCOL)); > > > > >>>>> NewNetworkInterfaceInstalled = FALSE; > > > > >>>>> Index = 0; > > > > >>>>> - do { > > > > >>>>> + > > > > >>>>> + // Get IP Type to filter out unnecessary network protocol > > if possible > > > > >>>>> + IpType = GetHiIpProtocolType (); > > > > >>>>> + > > > > >>>>> + for (Index = 0; Index < ListCount; Index++) { > > > > >>>>> + // Check IP Type and skip an unnecessary network > > protocol if does > > > > >> 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 = 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 == ProtocolTypeTcp6)) > > > > >>>>> +#define VALID_TCP4(TargetNetworkInterface, > > ThisNetworkInterface) > > > > >>> (!TargetNetworkInterface->IsIpv6 && (ThisNetworkInterface- > > > > >>>> NetworkProtocolType == ProtocolTypeTcp4)) > > > > >>>>> +#define IS_TCP4_MATCH(IpType) > > > > >>> ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp4) > > && > > > > >> (IpType != > > > > >>> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4)) > > > > >>>>> +#define IS_TCP6_MATCH(IpType) > > > > >>> ((gRequiredProtocol[Index].ProtocolType == ProtocolTypeTcp6) > > && > > > > >> (IpType != > > > > >>> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)) > > > > >>>> > > > > >>>> And these macros to test for ==, not != > > > > >>>> > > > > >>>> > > > > >>>> Igor: First version tested "==", but we agreed that it may > > not work 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, > > > > but > > > > >>>> 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 == > > > > >>> > REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_UNKNOWN) > > > { > > > > >>> // The protocol type is not specified in SMBIOS table type > > 42h > > > > >>> return EFI_UNSUPPORTED; > > > > >>> } > > > > >>> > > > > >>> for (Index = 0; Index < ListCount; Index++) { > > > > >>> if ((gRequiredProtocol[Index].ProtocolType == > > ProtocolTypeTcp4) && > > > > >>> (IpType != > > > > >> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP4)) { > > > > >>> continue; > > > > >>> } > > > > >>> if ((gRequiredProtocol[Index].ProtocolType == > > ProtocolTypeTcp6) && > > > > >>> (IpType != > > > > >> REDFISH_HOST_INTERFACE_HOST_IP_ADDRESS_FORMAT_IP6)) { > > > > >>> continue; > > > > >>> } > > > > >>> <skip> > > > > >>> > > > > >>> Regards, > > > > >>> Mike. > > > > > > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111295): https://edk2.groups.io/g/devel/message/111295 Mute This Topic: https://groups.io/mt/102618505/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
