On 2/27/24 05:49, Sivaraman Nainar wrote:
> Hi Laszlo,
>
> We can see the issue not only with SLES, it can be seen with Ubuntu 22 also.
>
> Do we have any channel to work with grub team to fix this issue?
No particular channel. Oliver has been participating in upstream grub2
development (CC'd), so I figure bug analysis and bugfix posting should
occur on their normal development mailing list.
Laszlo
> -----Original Message-----
> From: Sivaraman Nainar
> Sent: Monday, February 26, 2024 4:01 PM
> To: devel@edk2.groups.io; Sivaraman Nainar <sivaram...@ami.com>; Laszlo Ersek
> <ler...@redhat.com>; Santhosh Kumar V <santhoshkum...@ami.com>; Saloni
> Kasbekar <saloni.kasbe...@intel.com>; Zachary Clark-williams
> <zachary.clark-willi...@intel.com>
> Cc: Raj V Akilan <ra...@ami.com>; Soundharia R <soundhar...@ami.com>
> Subject: RE: [EXTERNAL] Re: [edk2-devel] [PATCH] NetworkPkg:Resolved
> Consecutive Pxe-Http Boot Issue
>
> @Saloni Kasbekar, @Zachary Clark-williams,
>
> Could you please add your feedback on the changes proposed?
>
> Thanks
> Siva
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sivaraman
> Nainar via groups.io
> Sent: Thursday, February 22, 2024 7:33 AM
> To: Laszlo Ersek <ler...@redhat.com>; devel@edk2.groups.io; Santhosh Kumar V
> <santhoshkum...@ami.com>; Saloni Kasbekar <saloni.kasbe...@intel.com>;
> Zachary Clark-williams <zachary.clark-willi...@intel.com>
> Cc: Raj V Akilan <ra...@ami.com>; Soundharia R <soundhar...@ami.com>
> Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] NetworkPkg:Resolved Consecutive
> Pxe-Http Boot Issue
>
>
> **CAUTION: The e-mail below is from an external source. Please exercise
> caution before opening attachments, clicking links, or following guidance.**
>
> Laszlo:
>
> Thanks for the detailed feedback on the changes for this issue. Since we are
> not sure if this change are valid / violate some purpose of SNP driver, it
> mentioned as Workaround.
>
> @Saloni Kasbekar and @Clark-williams, Zachary can add more on these changes.
>
> As you recommended, we can have PCD which controls these changes till the
> changes are addressed in grub.
>
> @Santhosh Kumar V is this issue can be seen only in SLES 15 or it can be
> found in any OS having Grub 2.x?
>
> Thanks
> Siva
> -----Original Message-----
> From: Laszlo Ersek <ler...@redhat.com>
> Sent: Thursday, February 22, 2024 5:15 AM
> To: devel@edk2.groups.io; Santhosh Kumar V <santhoshkum...@ami.com>
> Cc: Sivaraman Nainar <sivaram...@ami.com>; Raj V Akilan <ra...@ami.com>;
> Soundharia R <soundhar...@ami.com>; Saloni Kasbekar
> <saloni.kasbe...@intel.com>; Zachary Clark-williams
> <zachary.clark-willi...@intel.com>
> Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] NetworkPkg:Resolved Consecutive
> Pxe-Http Boot Issue
>
>
> **CAUTION: The e-mail below is from an external source. Please exercise
> caution before opening attachments, clicking links, or following guidance.**
>
> On 2/21/24 18:15, Santhosh Kumar V via groups.io wrote:
>> The customer has a server environment where PXE and HTTP service run in same
>> Linux Server. In this environment a SUT trying to boot to SLES 15 OS via PXE
>> from the Boot Menu. After PXE Boot file downloaded and grub Loaded without
>> continuing for installation Exit is pressed and control back to Setup.
>> Now the HTTP boot to SLES 15 OS tried in the same environment and failed to
>> download the file. If there is a reconnect -r performed before this HTTP
>> Boot then boot file download and installation is getting success.
>> Root cause of the issue is, when Exit from grub performed, boot Loader Stops
>> the SNP Driver and starts the same.
>
> This sentence feels like the key one.
>
> Are you saying that grub calls Snp->Start() just before it exits?
>
> If so, am I right to suspect that that's a grub bug? It sounds like a
> resource leak, after all.
>
> Can you perhaps include a grub source code location / pointer in the commit
> message?
>
>> During this process SNP is in Initialized State. When HTTP boot is performed
>> immediately after PXE Failure, the MNP configure method initiates the SNP
>> Start again. Since the SNP already started by grub it returns
>> EFI_ALREADY_STARTED and none of the upper Layer drivers are getting started.
>> As a work around in MNPConfigure(), if the SNP Start failed with Already
>> Started and in Initialized state we can return success so that rest of the
>> drivers can be loaded and HTTP boot can work.
>>
>>
>> Cc: Saloni Kasbekar <saloni.kasbe...@intel.com>
>> Cc: Zachary Clark-williams <zachary.clark-willi...@intel.com>
>>
>> Signed-off-by: SanthoshKumar <santhoshkum...@ami.com>
>> ---
>> NetworkPkg/MnpDxe/MnpConfig.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/NetworkPkg/MnpDxe/MnpConfig.c
>> b/NetworkPkg/MnpDxe/MnpConfig.c index 93587d53aa..0f2df28d73 100644
>> --- a/NetworkPkg/MnpDxe/MnpConfig.c
>> +++ b/NetworkPkg/MnpDxe/MnpConfig.c
>> @@ -1120,7 +1120,9 @@ MnpStartSnp (
>> // Start the simple network.
>>
>> //
>>
>> Status = Snp->Start (Snp);
>>
>> -
>>
>> + if ((Status == EFI_ALREADY_STARTED ) && (Snp->Mode->State ==
>> + EfiSimpleNetworkInitialized)) {
>>
>> + return EFI_SUCCESS;
>>
>> + }
>>
>> if (!EFI_ERROR (Status)) {
>>
>> //
>>
>> // Initialize the simple network.
>>
>
> The commit message does say this is a workaround, and I don't immediately any
> see why this workaround (in the code) would be problematic in practice, but
> it still leaves a bad taste in my mouth.
>
> Consider: the call path is the following:
>
> MnpConfigure() [NetworkPkg/MnpDxe/MnpConfig.c] -- public
> .Configure() protocol member function
> MnpConfigureInstance() [NetworkPkg/MnpDxe/MnpConfig.c]
> MnpStart() [NetworkPkg/MnpDxe/MnpConfig.c]
> // see notes!
> MnpStartSnp() [NetworkPkg/MnpDxe/MnpConfig.c]
>
> Notes: the MnpStartSnp() call in MnpStart() is conditional on two
> circumstances (at the same time):
> - "If it's not a configuration update, increase the configured children
> number."
> - "It's the first configured child, start the simple network."
>
> In other words, the MNP driver has just bound SNP "BY_DRIVER" (i.e.,
> exclusively), installed the MNP service binding protocol for each vlan
> (IIUC), and one of those SB instances is now being used to create the first
> MNP instance. I think that under these circumstances, it is reasonable for
> the MNP driver to expect that the underlying SNP be in stopped state. :/
>
> How long would NetworkPkg have to carry this workaround? (I.e., how long
> before the grub issue is fixed, and the buggy version deprecated?)
>
> I'd prefer at least a comment in the code that the return path is a
> workaround for (I feel) an earlier SNP usage violation.
>
> A FeaturePCD to disable the workaround could be reasonable too (but the
> NetworkPkg maintainers could disagree about that).
>
>
> BTW, the commit message should be wrapped at 75 characters. These long lines
> (in the body) will pass PatchCheck, but generate warnings. Those warnings are
> tolerable for log quotes, URLs, etc, but for normal English text, wrapping is
> much preferred.
>
>
> Another comment on the commit message: the subject line should state
> something like
>
> NetworkPkg/MnpDxe: work aroung SNP state leak in grub
>
> Laszlo
>
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is intended to
> be read only by the individual or entity to whom it is addressed or by their
> designee. 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 telephone
> at 770-246-8600, and then delete or destroy all copies of the transmission.
>
>
>
>
>
> -The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI). This communication is intended to
> be read only by the individual or entity to whom it is addressed or by their
> designee. 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 telephone
> at 770-246-8600, and then delete or destroy all copies of the transmission.
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116033): https://edk2.groups.io/g/devel/message/116033
Mute This Topic: https://groups.io/mt/104498511/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
