Re: edk2-stable202311 Code freeze process violation Re: [edk2-devel] [PATCH v5 2/2] RedfishPkg: RedfishDiscoverDxe: Optimize the Redfish Discover flow

["Michael D Kinney" <michael.d.kinney@intel.com>]

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-organizations-repositories/managing-repository-roles/repository-roles-for-an-organization

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.Chang@amd.com>
> Sent: Wednesday, November 15, 2023 3:55 AM
> To: Leif Lindholm <quic_llindhol@quicinc.com>; devel@edk2.groups.io;
> Mike Maslenkin <mike.maslenkin@gmail.com>; igork@ami.com; Gao, Liming
> <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Nickle Wang <nicklew@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_llindhol@quicinc.com>; devel@edk2.groups.io;
> Mike
> > Maslenkin <mike.maslenkin@gmail.com>; igork@ami.com; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Nickle Wang <nicklew@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_llindhol@quicinc.com>
> > > Sent: Wednesday, November 15, 2023 6:59 PM
> > > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Mike
> > > Maslenkin <mike.maslenkin@gmail.com>; igork@ami.com; Gao, Liming
> > > <gaoliming@byosoft.com.cn>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Cc: Nickle Wang <nicklew@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.maslenkin@gmail.com>;
> > devel@edk2.groups.io;
> > > >> igork@ami.com
> > > >> Cc: Leif Lindholm <quic_llindhol@quicinc.com>; Nickle Wang
> > > >> <nicklew@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.maslenkin@gmail.com>
> > > >>> Sent: Wednesday, November 15, 2023 7:53 AM
> > > >>> To: devel@edk2.groups.io; igork@ami.com
> > > >>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>; Chang, Abner
> > > >>> <Abner.Chang@amd.com>; Nickle Wang <nicklew@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.com@groups.io> wrote:
> > > >>>>
> > > >>>> Hi Leif,
> > > >>>> Please see my comments below.
> > > >>>> Thank you,
> > > >>>> Igor
> > > >>>>
> > > >>>>
> > > >>>> -----Original Message-----
> > > >>>> From: Leif Lindholm <quic_llindhol@quicinc.com>
> > > >>>> Sent: Tuesday, November 14, 2023 12:26 PM
> > > >>>> To: devel@edk2.groups.io; Igor Kulchytskyy <igork@ami.com>
> > > >>>> Cc: Abner Chang <abner.chang@amd.com>; Nickle Wang
> > > >>> <nicklew@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.chang@amd.com>
> > > >>>>> Cc: Nickle Wang <nicklew@nvidia.com>
> > > >>>>> Signed-off-by: Igor Kulchytskyy <igork@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 (#111283): https://edk2.groups.io/g/devel/message/111283
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/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-