public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Ye, Ting" <ting.ye@intel.com>,
	"Carsey, Jaben" <jaben.carsey@intel.com>,
	 "Fu, Siyuan" <siyuan.fu@intel.com>,
	"Shao, Ming" <ming.shao@intel.com>,
	"Li, Ruth" <ruth.li@intel.com>
Subject: Re: [Patch 0/5] Support windowsize to benefit tftp/pxe download performance.
Date: Thu, 20 Sep 2018 05:54:50 +0000	[thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B7274164A134D@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <7ec63ed7-e969-efdf-3230-48c8e9274ec4@redhat.com>

Hi Laszlo, 

I agree there is no document to describe the detailed difference against the overlapped network drivers the between NetworkPkg and MdeModulePkg (except IPv4/IPv6 support ). We only declared that those drivers should not be used at the same (https://github.com/tianocore/tianocore.github.io/wiki/NetworkPkg-Getting-Started-Guide). 

Actually, the problem you mentioned here only exists in ISCSI/TCP/PXE drivers - Tcp4Dxe VS TcpDxe, IScsiDxe VS IScsiDxe,  UefiPxeBcDxe VS UefiPxeBcDxe. So, as you said, it's the time for us to add some declaration somewhere (inf or wiki) -- For those three drivers in MdeModulePkg,  they will remain unchanged until there is any specific requirement that we need fix any issue. That will greatly reduce the effort to maintain/test those combine of two drivers.  So, we don’t recommend to use those three drivers in MdeModulePkg because they might some issues, which has been fixed in the NetworkPkg drivers. If you agree, we will add some statement in the corresponding *.inf files.

Thanks,
Jiaxin




> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, September 19, 2018 7:25 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>;
> Fu, Siyuan <siyuan.fu@intel.com>; Shao, Ming <ming.shao@intel.com>
> Subject: Re: [edk2] [Patch 0/5] Support windowsize to benefit tftp/pxe
> download performance.
> 
> 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 <lersek@redhat.com>
> >>
> >
> > 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


  reply	other threads:[~2018-09-20  5:55 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-17  5:43 [Patch 0/5] Support windowsize to benefit tftp/pxe download performance Jiaxin Wu
2018-09-17  5:43 ` [Patch 1/5] MdeModulePke/Mtftp4Dxe: Support windowsize in read request operation Jiaxin Wu
2018-09-17  5:43 ` [Patch 2/5] NetworkPkg/Mtftp6Dxe: " Jiaxin Wu
2018-09-17  5:43 ` [Patch 3/5] ShellPkg/TftpDynamicCommand: Add one option for tftp command to specify windowsize Jiaxin Wu
2018-09-17 22:18   ` Carsey, Jaben
2018-09-17  5:43 ` [Patch 4/5] MdeModulePkg/MdeModulePkg.dec: Define one PCD for PXE to specify MTFTP windowsize Jiaxin Wu
2018-09-18 11:04   ` Laszlo Ersek
2018-09-19  1:55     ` Wu, Jiaxin
2018-09-19  0:38   ` Fu, Siyuan
2018-09-19  2:21     ` Wu, Jiaxin
2018-09-17  5:43 ` [Patch 5/5] NetworkPkg/UefiPxeBcDxe: Use the specified " Jiaxin Wu
2018-09-18 11:23 ` [Patch 0/5] Support windowsize to benefit tftp/pxe download performance Laszlo Ersek
2018-09-19  2:20   ` Wu, Jiaxin
2018-09-19 11:24     ` Laszlo Ersek
2018-09-20  5:54       ` Wu, Jiaxin [this message]
     [not found]         ` <845df55b-b9ba-20f9-25fd-22778253c198@redhat.com>
2018-09-21  6:33           ` Wu, Jiaxin
2018-09-21  9:37             ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=895558F6EA4E3B41AC93A00D163B7274164A134D@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox