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 A196D941D93 for ; Wed, 21 Feb 2024 23:45:20 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=pxkfz4JiYuvYzLmdpd5FV++Nd6fY6kziUkLv7or+tVk=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version: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=1708559119; v=1; b=wneiiL1BLofk0VthZggdYqQAdOOQn/ndPSAqdoOZORuOctgPhxLoezVMSGYroSPl88xYbVpn nne6LZu8irACk8f4eGWlIClBonkUMbG3fdmbguzLIod4yMvdPneD3jkIMJavQ2tIMf5kRFmQ+9l Lh9EPpy0MhS/AoCZtJ5mt+Hk= X-Received: by 127.0.0.2 with SMTP id Pa8tYY7687511xObfQUuDO6J; Wed, 21 Feb 2024 15:45:19 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web11.1479.1708559118626260947 for ; Wed, 21 Feb 2024 15:45:18 -0800 X-Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-519-tGSi0EFROxOIGFxewt5jKQ-1; Wed, 21 Feb 2024 18:45:14 -0500 X-MC-Unique: tGSi0EFROxOIGFxewt5jKQ-1 X-Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D4600386914D; Wed, 21 Feb 2024 23:45:13 +0000 (UTC) X-Received: from [10.39.192.46] (unknown [10.39.192.46]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A02132166B32; Wed, 21 Feb 2024 23:45:12 +0000 (UTC) Message-ID: <21b2d17d-5de5-5f08-ef6b-a0a285b05917@redhat.com> Date: Thu, 22 Feb 2024 00:45:11 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH] NetworkPkg:Resolved Consecutive Pxe-Http Boot Issue To: devel@edk2.groups.io, santhoshkumarv@ami.com Cc: Sivaraman Nainar , Raj V Akilan , Soundharia R , Saloni Kasbekar , Zachary Clark-williams References: <20240221171347.1343-1-santhoshkumarv@ami.com> From: "Laszlo Ersek" In-Reply-To: <20240221171347.1343-1-santhoshkumarv@ami.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: J5tA63kq48gXBN9t7oiFbfDbx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 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=wneiiL1B; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.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 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 s= ame Linux Server. In this environment a SUT trying to boot to SLES 15 OS vi= a PXE from the Boot Menu. After PXE Boot file downloaded and grub Loaded wi= thout 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 St= ops 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 perfor= med immediately after PXE Failure, the MNP configure method initiates the S= NP Start again. Since the SNP already started by grub it returns EFI_ALREAD= Y_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 > Cc: Zachary Clark-williams > > Signed-off-by: SanthoshKumar > --- > 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 =3D Snp->Start (Snp); > > - > > + if ((Status =3D=3D EFI_ALREADY_STARTED ) && (Snp->Mode->State =3D=3D E= fiSimpleNetworkInitialized)) { > > + 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 .Configu= re() 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 num= ber." - "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 -=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 (#115755): https://edk2.groups.io/g/devel/message/115755 Mute This Topic: https://groups.io/mt/104498511/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-