From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A4FD021B02822 for ; Wed, 19 Sep 2018 04:25:01 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2882D3084219; Wed, 19 Sep 2018 11:25:01 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-175.rdu2.redhat.com [10.10.120.175]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9454189F59; Wed, 19 Sep 2018 11:24:59 +0000 (UTC) To: "Wu, Jiaxin" , "edk2-devel@lists.01.org" Cc: "Ye, Ting" , "Carsey, Jaben" , "Fu, Siyuan" , "Shao, Ming" References: <20180917054348.19228-1-Jiaxin.wu@intel.com> <895558F6EA4E3B41AC93A00D163B727416494122@SHSMSX103.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: <7ec63ed7-e969-efdf-3230-48c8e9274ec4@redhat.com> Date: Wed, 19 Sep 2018 13:24:58 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <895558F6EA4E3B41AC93A00D163B727416494122@SHSMSX103.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.40]); Wed, 19 Sep 2018 11:25:01 +0000 (UTC) Subject: Re: [Patch 0/5] Support windowsize to benefit tftp/pxe download performance. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Sep 2018 11:25:01 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/19/18 04:20, Wu, Jiaxin wrote: >> On 09/17/18 07:43, Jiaxin Wu wrote: >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=886 >>> >>> The series patches are to support the TFTP windowsize option described in >> RFC 7440. >>> TFTP shell command and UEFI PXE driver will use the feature to benefit the >> download >>> performance. >> >> I tested this series, between two virtual machines running on my laptop. >> The TFTP server program that I used was "tftp-server-5.2-22.el7.x86_64". >> The downloaded file was 478,150,656 bytes in size. I built OVMF with >> NETWORK_IP6_ENABLE, so that the last patch would take effect for both >> PXEv4 and PXEv6. >> >> Before the series: >> - PXEv4: 75 seconds (~ 6225 KB/s) >> - PXEv6: 100 seconds (~ 4669 KB/s) >> >> After the series: >> - PXEv4: 48 seconds (~ 9728 KB/s) >> - PXEv6: 60 seconds (~ 7782 KB/s) >> >> These measurements are very rough (I didn't run them multiple times >> etc), but I think they are still quite good indicators. >> >> For the testing, I used the UEFI boot options in UiApp, and not the >> shell command, hence I have no feedback on patch #3. >> >> For patches #1, #2, and #5: >> >> Tested-by: Laszlo Ersek >> > > Thanks the verification. > > >> However, as I pointed out elsewhere in the thread, I think: >> >> - You might want to port the changes from patch#5 to >> "MdeModulePkg/Universal/Network/UefiPxeBcDxe" as well, in a separate >> patch (patch #6). >> >> - If not, then (a) we should document this feature difference in the INF >> files of the UefiPxeBcDxe drivers, (b) patch #4 should be re-done so >> that it target NetworkPkg, not MdeModulePkg. >> > > As I said in the previous email, normally, we only add the new feature into the combo driver. But I think that's depends on the request. If you want to include the feature into MdeModulePkg/Universal/Network/UefiPxeBcDxe since the OVMF platform might use it, I will create patch #6. If not, I will follow the comments (a)/(b). I don't currently have a use case or "requirement" for the window size feature to work in an OVMF build that does *not* have NETWORK_IP6_ENABLE. So from my side, we can delay the porting until such a need materializes (it might never materialize, of course!) However, because this information -- i.e., the feature separation between the IPv4-only, and the combined IPv4/IPv6 driver -- is new to me, can we please document it somewhere in the code, for example, in the INF files? We don't have to spell out the TFTP window size feature by name, just the fact that NetworkPkg's UefiPxeBcDxe has more features in general (even such features that are orthogonal to internet protocol version, v4 vs. v6). If such documentation already exists, then I missed it, sorry! Thanks! Laszlo >> - Patch #4 (regardless of package DEC) should be extended with >> documentation (both DEC and UNI). >> >> Thanks >> Laszlo