Hi Laszlo, Thanks for your comments first. Ip4Dxe driver update patch has been sent out and reviewed, we have submitted to edk2 already. So, that means the packages I listed will consume Ip4Config2 protocol integrated in Ip4Dxe driver directly. For patch series, those patches used to update related pkgs and remove Ip4ConfigDxe driver. I will submit related pkgs to edk2 first, and then remove Ip4ConfigDxe driver. I don't think the patch series has any issue.
Thanks. Jiaxin -----Original Message----- From: Laszlo Ersek [mailto:ler...@redhat.com] Sent: Tuesday, July 7, 2015 8:56 PM To: Wu, Jiaxin Cc: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] [Patch 3/7] OvmfPkg: Remove Ip4ConfigDxe module from OvmfPkg. On 07/07/15 03:27, jiaxinwu wrote: > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: jiaxinwu <jiaxin...@intel.com> > --- > OvmfPkg/OvmfPkgIa32.dsc | 1 - > OvmfPkg/OvmfPkgIa32.fdf | 1 - > OvmfPkg/OvmfPkgIa32X64.dsc | 1 - > OvmfPkg/OvmfPkgIa32X64.fdf | 1 - > OvmfPkg/OvmfPkgX64.dsc | 1 - > OvmfPkg/OvmfPkgX64.fdf | 1 - > 6 files changed, 6 deletions(-) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index > bb008f6..c3508e6 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -524,11 +524,10 @@ > MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf > MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf > MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf > MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf > MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf > - MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.inf > MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf > MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf > MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf > !if $(NETWORK_IP6_ENABLE) == TRUE > NetworkPkg/Ip6Dxe/Ip6Dxe.inf > diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf index > 24ad0bf..aadb099 100644 > --- a/OvmfPkg/OvmfPkgIa32.fdf > +++ b/OvmfPkg/OvmfPkgIa32.fdf > @@ -302,11 +302,10 @@ FILE FREEFORM = > PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) { > INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf > INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf > INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf > INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf > INF MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf > - INF MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.inf > INF MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf > INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf > INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf > !if $(NETWORK_IP6_ENABLE) == TRUE > INF NetworkPkg/Ip6Dxe/Ip6Dxe.inf > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index fed584f..40a1cf1 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -531,11 +531,10 @@ > MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf > MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf > MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf > MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf > MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf > - MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.inf > MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf > MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf > MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf > !if $(NETWORK_IP6_ENABLE) == TRUE > NetworkPkg/Ip6Dxe/Ip6Dxe.inf > diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf > index 5f98be8..bc42a2b 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.fdf > +++ b/OvmfPkg/OvmfPkgIa32X64.fdf > @@ -302,11 +302,10 @@ FILE FREEFORM = > PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) { > INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf > INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf > INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf > INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf > INF MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf > - INF MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.inf > INF MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf > INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf > INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf > !if $(NETWORK_IP6_ENABLE) == TRUE > INF NetworkPkg/Ip6Dxe/Ip6Dxe.inf > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index > 92af10f..ac4e731 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -529,11 +529,10 @@ > MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf > MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf > MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf > MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf > MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf > - MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.inf > MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf > MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf > MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf > !if $(NETWORK_IP6_ENABLE) == TRUE > NetworkPkg/Ip6Dxe/Ip6Dxe.inf > diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf index > 11e74e7..b6992bd 100644 > --- a/OvmfPkg/OvmfPkgX64.fdf > +++ b/OvmfPkg/OvmfPkgX64.fdf > @@ -302,11 +302,10 @@ FILE FREEFORM = > PCD(gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile) { > INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf > INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf > INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf > INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf > INF MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf > - INF MdeModulePkg/Universal/Network/Ip4ConfigDxe/Ip4ConfigDxe.inf > INF MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf > INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf > INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf > !if $(NETWORK_IP6_ENABLE) == TRUE > INF NetworkPkg/Ip6Dxe/Ip6Dxe.inf > I don't see any driver in the edk2 tree that installs gEfiIp4Config2ProtocolGuid, at the moment. In your other email you wrote: > Ip4ConfigDxe driver is deprecated in UEFI 2.5, so we will not support > original Ip4Config Protocol, which is replace by Ip4Config2 Protocol > integrated in Ip4Dxe driver. > Related pkgs(MdeModulePkg, Nt32Pkg, ArmPlatformPkg, ArmVirtPkg, > EmulatorPkg, OvmfPkg, Vlv2TbltDevicePkg) need to remove Ip4ConfigDxe > module and related guid definitions, otherwise they will meet build > failure issue after we update Ip4Dxe driver and deprecate Ip4ConfigDxe > driver. However, if these patches were committed first, then the packages you listed would have no Ip4Config protocol *at all*, either version 1 or version 2. Because, as you said, Ip4Config2 will only be implemented in the future (in Ip4Dxe). That would break IPv4 networking for all of these packages: Ip4Dxe.inf lists "gEfiIp4ConfigProtocolGuid" (that is, version 1) as TO_START. The right way to implement such changes is the following: - First implement the new (version 2) protocol in the edk2 tree. This can be a separate, initial patch set, or else the first segment of a larger patch series. - Then flip client packages over to the new protocol version. This is where the old driver INF can be removed from the DSC and FDF files. - Finally, you can delete Ip4ConfigDxe from the tree completely. In other words, the tree must build, and preferably even work, at *any* stage within your patch series. The patches must preserve bisectability. Therefore, I don't think this patch series is good, in its current form. Please consider the pattern I described above. Thanks Laszlo ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
