Re: DHCP Process Starts Even there is NO Media Connected

[Karunakar P <karunakarp@amiindia.co.in>]

Please review the patch

[Patch] MdeModulePkg/Universal/Network/Dhcp4Dxe: Implantation of handling EFI_NO_MEDIA status code for DHCP4/6.Start()
NetworkPkg/Dhcp6Dxe :  Implantation of handling EFI_NO_MEDIA status code for DHCP4/6.Start()

MdeModulePkg/Universal/Network/Dhcp4Dxe/ Dhcp4Impl.c | 11
NetworkPkg/Dhcp6Dxe/ Dhcp6Impl.c |  11
2 files changed, 22 insertions(+)

a. MdeModulePkg/Universal/Network/Dhcp4Dxe/ Dhcp4Impl.c
b. NetworkPkg/Dhcp6Dxe/ Dhcp6Impl.c


a@ EfiDhcp4Start (
  IN EFI_DHCP4_PROTOCOL     *This,
  IN EFI_EVENT              CompletionEvent   OPTIONAL
  )
{
  DHCP_PROTOCOL             *Instance;
  DHCP_SERVICE              *DhcpSb;
  EFI_STATUS                Status;
  EFI_TPL                   OldTpl;
+ BOOLEAN                   MediaPresent;
.
.
.
OldTpl  = gBS->RaiseTPL (TPL_CALLBACK);
 DhcpSb  = Instance->Service;
  
+  //
+ // Check media status before DHCP Start.
+  //
+  MediaPresent = TRUE;
+  NetLibDetectMedia (DhcpSb->Controller, &MediaPresent);
+  if (!MediaPresent) {
+    Status = EFI_NO_MEDIA;
+	DEBUG((DEBUG_ERROR,"\nIn EfiDhcp4Start  MediaPresent Status = %r\n",Status));
+    goto ON_ERROR;
+ }

  if (DhcpSb->DhcpState == Dhcp4Stopped) {
    Status = EFI_NOT_STARTED;
    goto ON_ERROR;
  }
.
.
.
}


b @ EfiDhcp6Start (
  IN EFI_DHCP6_PROTOCOL        *This
  )
{
  EFI_STATUS                   Status;
  EFI_TPL                      OldTpl;
  DHCP6_INSTANCE               *Instance;
  DHCP6_SERVICE                *Service;
+  BOOLEAN                      MediaPresent;
.
.
.
OldTpl           = gBS->RaiseTPL (TPL_CALLBACK);
Instance->UdpSts = EFI_ALREADY_STARTED;

+  //
+  // Check media status before DHCP Start.
+  //
+  MediaPresent = TRUE;
+  NetLibDetectMedia (Service->Controller, &MediaPresent);
+  if (!MediaPresent) {
+    Status = EFI_NO_MEDIA;
+    DEBUG((DEBUG_ERROR,"\nIn EfiDhcp6Start  MediaPresent Status = %r\n",Status));
+    goto ON_ERROR;
+  }
  
  //
  // Send the solicit message to start S.A.R.R process.
  //
  Status = Dhcp6SendSolicitMsg (Instance);
.
.
.
}


Thanks,
Karunakar

-----Original Message-----
From: Karunakar P 
Sent: Friday, December 01, 2017 11:10 AM
To: 'Wu, Jiaxin'; Richardson, Brian; Fu, Siyuan; Ye, Ting; 'edk2-devel@lists.01.org'
Subject: RE: DHCP Process Starts Even there is NO Media Connected

Hi Jiaxin,

I've updated the patch changes for Dhcp6Dxe driver also and attached the same for review.

Filed a tracker in TianoCore Bugzilla for this issue and attached the changes for review,
https://bugzilla.tianocore.org/show_bug.cgi?id=804

Thanks,
Karunakar

-----Original Message-----
From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
Sent: Friday, December 01, 2017 9:15 AM
To: Richardson, Brian; Karunakar P; Fu, Siyuan; Ye, Ting; 'edk2-devel@lists.01.org'
Subject: RE: DHCP Process Starts Even there is NO Media Connected

Hi Karunakar,

Per your question #3.c, I think it's *unreasonable* to check Media Status for EFI_IP4_CONFIG2_SET_DATA. 

Even there is no media connected, we still need to set the data instead of return directly. If the EFI_IP4_CONFIG2_SET_DATA.SetData() is to set the DHCP policy, the SetData() interface will try to do the DHCP process to get one valid default address, but if there is any failure happen in DHCP process (e.g. no media connected), we should continue change the policy to DHCP and return EFI_SUCCESS, which align with static policy. So, I don't prefer to check the Media Status in EFI_IP4_CONFIG2_SET_DATA.SetData(). 

I have reviewed your patch, the Dhcp4Dxe update is good to me. Can you send out the formal patch to the EDK2 community for the review or need us do to that? Note: don't forget to update the Dhcp6Dxe driver. 

Also thanks Brain's reminder, please file a tracker in TianoCore Bugzilla for this issue.

Thanks,
Jiaxin

> -----Original Message-----
> From: Richardson, Brian
> Sent: Thursday, November 30, 2017 11:02 PM
> To: Karunakar P <karunakarp@amiindia.co.in>; Wu, Jiaxin 
> <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting 
> <ting.ye@intel.com>; 'edk2-devel@lists.01.org'
> <edk2-devel@lists.01.org>
> Subject: RE: DHCP Process Starts Even there is NO Media Connected
> 
> We saw some problems running Linux UEFI Validation (LUV) at the last 
> UEFI Plugfest that are probably related to this issue. At the time we 
> asked the LUV team to investigate it as a test issue, but it may 
> actually be a stack problem based on this information.
> 
> Have you filed a tracker in TianoCore Bugzilla for this issue?
> 
> Thanks ... br
> ---
> Brian Richardson, Senior Technical Marketing Engineer, Intel Software 
> brian.richardson@intel.com -- @intel_brian (Twitter & WeChat) 
> https://software.intel.com/en-us/meet-the-developers/evangelists/team/
> brian-
> richardson
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Karunakar P
> Sent: Thursday, November 30, 2017 3:59 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan 
> <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; 
> 'edk2-devel@lists.01.org' <edk2-devel@lists.01.org>
> Subject: Re: [edk2] DHCP Process Starts Even there is NO Media 
> Connected
> 
> Hi Jiaxin,
> 
> Please find my below comments/suggestions.
> 
> 
> 1.       Yes, Current DHCP4/6 Start() implementation doesn't check for Media
> Status. It will be better to implement it in order to sync with UEFI spec.
> 
> 2.       DHCP process may be initiated by HTTP/PXE/ISCSI or Assigning IP
> (SetData) from Shell or using BIOS setup page.
> 
> HTTP,PXE and ISCSI is already Checking Media Presence before DHCP 
> Start, So it will have NO effect if we do implementation in DHCP4/6 Start().
> 
> 3.       Current implementation of EFI_IP4_CONFIG2_SET_DATA, also NOT
> handling checking Media Status.
> 
> a.       UEFI Spec NOT defines EFI_NO_MEDIA status code for
> EFI_IP4_CONFIG2_SET_DATA, I'm NOT sure what's reason behind it or 
> might be missing.
> 
> b.      UEFI Spec defines EFI_DEVICE_ERROR status code for
> EFI_IP4_CONFIG2_SET_DATA, If we can use the same status for Media 
> presence then no issues.
> 
> c.       When there is No Media connected and if we try to assign IP over
> DHCP(SetData), I guess there is no need to proceed further in 
> EfiIp4Config2SetData and we can return with EFI_DEVICE_ERROR.
> 
> Based on above points(1 & 3.c ), I've updated the suggested changes 
> and attached the same (CheckMediaStatus_V2.rar)
> 
> Could you please review and provide your comments.
> Please correct  if anything wrong.
> 
> Thank You,
> Karunakar
> 
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Wednesday, November 29, 2017 11:57 AM
> To: Karunakar P; Fu, Siyuan; Ye, Ting
> Subject: RE: DHCP Process Starts Even there is NO Media Connected
> 
> Hi Karunakar,
> 
> After talk with Siyuan, we agree it's reasonable to check the Media 
> status before starting DHCP process, but we'd better check it in DHCP 
> layer since the UEFI spec defines EFI_NO_MEDIA status code for 
> DHCP4/6.Start(), but our current implementation doesn't check it.
> 
> What do you think?
> 
> Thanks,
> Jiaxin
> 
> 
> 
> From: Karunakar P [mailto:karunakarp@amiindia.co.in]
> Sent: Tuesday, November 28, 2017 11:18 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Fu, 
> Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Ye, Ting 
> <ting.ye@intel.com<mailto:ting.ye@intel.com>>
> Subject: RE: DHCP Process Starts Even there is NO Media Connected
> 
> Could you please review the attachment changes for this support.
> 
> Thanks,
> Karunakar
> 
> From: Karunakar P
> Sent: Monday, November 27, 2017 12:53 PM
> To: 'Wu, Jiaxin'; 'Fu, Siyuan'; 'Ye, Ting'
> Subject: RE: DHCP Process Starts Even there is NO Media Connected
> 
> Could you please provide your comments...
> 
> Thank You,
> Karunakar
> 
> From: Karunakar P
> Sent: Thursday, November 23, 2017 2:05 PM
> To: 'Wu, Jiaxin'; Fu, Siyuan; Ye, Ting
> Subject: DHCP Process Starts Even there is NO Media Connected
> 
> Hello All,
> 
> When we try to Assign IP to SUT  using ifconfig command from Shell or
> IPv4 Network Configuration BIOS setup page DHCP process start even 
> there is no LAN cable connected to specific port.
> 
> Can we add a Media presence condition check before starting DHCP service?
> 
> Could you please correct if anything is wrong.
> 
> Thanks,
> Karunakar
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel