Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor TlsCreateChild

["Chang, Abner via groups.io" <abner.chang=amd.com@groups.io>]

[AMD Official Use Only - General]

Hi @saloni.kasbekar@intel.com,
Could you please check patch 1/6 and 3/6 in V2 and give your RB if there is no more concerns from you? Then I can merge this patch set. Thank you.

Abner


> -----Original Message-----
> From: Chang, Abner
> Sent: Sunday, January 7, 2024 8:08 PM
> To: devel@edk2.groups.io; saloni.kasbekar@intel.com; mcb30@ipxe.org
> Cc: Clark-williams, Zachary <zachary.clark-williams@intel.com>; Nickle Wang
> <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: RE: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor
> TlsCreateChild
>
> Hi Saloni,
> We still create TLS child via service binding protocol, the difference is we give
> HTTP handle to TLS service binding for installing TLS protocol on the HTTP
> handle instead of a on a new EFI handle.
> The logic is still the same just the code is moved around, I think we can just
> keep the function name the same..
>
> Thanks
> Abner
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Saloni
> > Kasbekar via groups.io
> > Sent: Saturday, January 6, 2024 5:32 AM
> > To: devel@edk2.groups.io; mcb30@ipxe.org; Chang, Abner
> > <Abner.Chang@amd.com>
> > Cc: Clark-williams, Zachary <zachary.clark-williams@intel.com>; Nickle Wang
> > <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> > Subject: Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor
> > TlsCreateChild
> >
> > Caution: This message originated from an External Source. Use proper
> caution
> > when opening attachments, clicking links, or responding.
> >
> >
> >      //
> >      // Check whether we need to create Tls child and open the TLS protocol.
> >      //
> > +    if (HttpInstance->UseHttps && !HttpInstance->TlsAlreadyCreated) {
> > +      // Create TLS child for this HTTP instance.
> > +      Status = TlsCreateChild (HttpInstance);
> > +      if (EFI_ERROR (Status)) {
> >          return EFI_DEVICE_ERROR;
> >        }
> >
> > Considering we're not creating TlsChildHandle anymore, we should probably
> > rename TlsCreateChild() to TlsCreateHandle(). Also update the comments
> > accordingly.
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> > Brown
> > Sent: Friday, January 5, 2024 9:12 AM
> > To: abner.chang@amd.com; devel@edk2.groups.io
> > Cc: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary
> > <zachary.clark-williams@intel.com>; Nickle Wang <nicklew@nvidia.com>;
> Igor
> > Kulchytskyy <igork@ami.com>
> > Subject: Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor
> > TlsCreateChild
> >
> > On 05/01/2024 08:37, abner.chang@amd.com wrote:
> > > From: Abner Chang <abner.chang@amd.com>
> > >
> > > - Use HTTP instance as the parameter for TlsCreateChild function.
> > > - Install TLS protocol on the HTTP instance thats create TLS child.
> >
> > Logic looks good to me, just some minor cosmetic comments.
> >
> > Commit title has "NetwokrPkg" typo, should be "NetworkPkg".
> >
> > > -  @return  The child handle with opened EFI_TLS_PROTOCOL and
> > EFI_TLS_CONFIGURATION_PROTOCOL.
> > > +  @return  EFI_SUCCESS        TLS child handle is returned in HttpInstance-
> > >TlsChildHandle
> > > +                              with opened EFI_TLS_PROTOCOL and
> > EFI_TLS_CONFIGURATION_PROTOCOL.
> >
> > Comment refers to TlsChildHandle, which no longer exists after this patch.
> >
> > > -  @return  The child handle with opened EFI_TLS_PROTOCOL and
> > EFI_TLS_CONFIGURATION_PROTOCOL.
> > > +  @return  EFI_SUCCESS        TLS child handle is returned in HttpInstance-
> > >TlsChildHandle
> > > +                              with opened EFI_TLS_PROTOCOL and
> > EFI_TLS_CONFIGURATION_PROTOCOL.
> >
> > As above.
> >
> > Reviewed-by: Michael Brown <mcb30@ipxe.org>
> >
> > Michael
> >
> >
> >
> >
> >
> >
> >
> >
> > 
> >



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113420): https://edk2.groups.io/g/devel/message/113420
Mute This Topic: https://groups.io/mt/103539578/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-