Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove ExitBootServices event

["Siyuan, Fu" <siyuan.fu@intel.com>]

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: 2019年10月11日 0:06
> To: Fu, Siyuan <siyuan.fu@intel.com>; devel@edk2.groups.io; Rabeda,
> Maciej <maciej.rabeda@intel.com>
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove
> ExitBootServices event
> 
> On 10/10/19 11:29, Fu, Siyuan wrote:
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: 2019年10月10日 16:06
> >> To: Fu, Siyuan <siyuan.fu@intel.com>; devel@edk2.groups.io; Rabeda,
> >> Maciej <maciej.rabeda@intel.com>
> >> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>
> >> Subject: Re: [edk2-devel] [PATCH v1 1/1] NetworkPkg/SnpDxe: Remove
> >> ExitBootServices event
> >>
> >> On 10/10/19 05:32, Fu, Siyuan wrote:
> >>> Hi, Maciej
> >>>
> >>> Considering that this patch has to co-work with corresponding UNDI
> device
> >> driver
> >>> bug fix, in order to avoid potential compatibility problem, please add a
> PCD
> >> to
> >>> NetworkPkg for this fix, and set the default value to disable state (no
> >> behavior
> >>>  change). The platform which need this fix could set the PCD to enable in
> >> its
> >>> platform DSC.
> >>
> >> Should the new PCD go into "NetworkPkg/NetworkPcds.dsc.inc", and be
> >> gated with a new build flag defined in
> "NetworkPkg/NetworkDefines.dsc.inc"?
> >
> > Currently not all the network package PCDs are listed in the
> NetworkPcds.dsc.inc,
> > But I do not oppose it if you think this PCD should be controlled by a build
> flag.
> 
> I mainly asked from a consistency point of view.
> 
> The inclusion of SnpDxe is already controlled by NETWORK_SNP_ENABLE, at
> least in platforms that utilize the NetworkPkg *.inc files. And the PCD
> in question would control the behavior of SnpDxe at ExitBootServices(),
> if I understand correctly.
> 
> This is similar to NETWORK_HTTP_BOOT_ENABLE and
> NETWORK_ALLOW_HTTP_CONNECTIONS:
> - The former includes HttpDxe and HttpBootDxe (and some other drivers).
> - The latter controls PcdAllowHttpConnections, which is consumed by
> HttpDxe and HttpBootDxe.
> 
> I don't feel too strongly about it, I just thought it worth raising.

It's not an existing "consistency" in current network package. The current macros
are mostly used for control more high level feature enable/disable, not such kind
of bug fix,
For example, NETWORK_ISCSI_ENABLE includes IScsiDxe driver, while 
PcdIScsiAIPNetworkBootPolicy is consumed by IScsiDriver, but not controlled by
a macro. And also other PXE, DHCP and PXE related PCDs.

Thanks,
Siyuan

> 
> Thanks
> Laszlo