* Re: [Patch] MdeModulePkg/NetLib: Add NetLibDetectMediaWaitTimeout() API to support EFI_NOT_READY media state detection
2017-11-27 8:11 [Patch] MdeModulePkg/NetLib: Add NetLibDetectMediaWaitTimeout() API to support EFI_NOT_READY media state detection fanwang2
@ 2017-12-04 1:12 ` Wu, Jiaxin
0 siblings, 0 replies; 2+ messages in thread
From: Wu, Jiaxin @ 2017-12-04 1:12 UTC (permalink / raw)
To: Wang, Fan, edk2-devel@lists.01.org; +Cc: Fu, Siyuan, Ye, Ting
My comments as below:
1. "ASSERT (MediaState != NULL)" should be removed since it will be checked later. The same issue is found in NetLibDetectMedia().
2. The Timer event should be created in the first loop for the second loop's check, otherwise, it will be invalid for the next CheckEvent().
do {
//
// Create new Timer
//
CreateEvent ()
SetTimer ();
do {
CheckEvent();
}
CloseEvent (Timer);
Timer = NULL;
}
3. How about changing the default value of *MediaState to EFI_SUCCESS? You know the error status may be returned from API, then the MediaState can't be updated correctly. In such a case, the MediaState should be treated as EFI_SUCCESS.
Or
We might need add the description to comments that the MediaState is only valid when EFI_SUCCESS returned from API.
Thanks,
Jiaxin
> -----Original Message-----
> From: Wang, Fan
> Sent: Monday, November 27, 2017 4:12 PM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan <siyuan.fu@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>; Wang, Fan <fan.wang@intel.com>
> Subject: [Patch] MdeModulePkg/NetLib: Add
> NetLibDetectMediaWaitTimeout() API to support EFI_NOT_READY media
> state detection
>
> In wireless connection, connecting state needs to be cared more
> about. ECR 1772 redefined the state EFI_NOT_READY to represent
> connecting state and can be retrieved from Aip protocol. This
> patch adds a new API to check media state at a specified time
> interval when network is connecting until the connection process
> finishes or timeout.
>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
> MdeModulePkg/Include/Library/NetLib.h | 40 +++++++
> MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 160
> +++++++++++++++++++++++++++
> MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf | 2 +
> 3 files changed, 202 insertions(+)
>
> diff --git a/MdeModulePkg/Include/Library/NetLib.h
> b/MdeModulePkg/Include/Library/NetLib.h
> index b9df46c..7862df9 100644
> --- a/MdeModulePkg/Include/Library/NetLib.h
> +++ b/MdeModulePkg/Include/Library/NetLib.h
> @@ -93,10 +93,16 @@ typedef UINT16 TCP_PORTNO;
> #define DNS_CLASS_INET 1
> #define DNS_CLASS_CH 3
> #define DNS_CLASS_HS 4
> #define DNS_CLASS_ANY 255
>
> +//
> +// Number of 100ns units time Interval for network media state detect
> +//
> +#define MEDIA_STATE_DETECT_TIME_INTERVAL 1000000U
> +
> +
> #pragma pack(1)
>
> //
> // Ethernet head definition
> //
> @@ -1246,10 +1252,44 @@ NetLibDetectMedia (
> IN EFI_HANDLE ServiceHandle,
> OUT BOOLEAN *MediaPresent
> );
>
> /**
> +
> + Detect media state for a network device. This routine will wait for a period
> of time at
> + a specified checking interval when a certain network is under connecting
> until connection
> + process finishes or timeout. If Aip protocol is supported by low layer
> drivers, three kinds
> + of media states can be detected: EFI_SUCCESS, EFI_NOT_READY and
> EFI_NO_MEDIA, represents
> + connected state, connecting state and no media state respectively. When
> function detects
> + the current state is EFI_NOT_READY, it will loop to wait for next time's
> check until state
> + turns to be EFI_SUCCESS or EFI_NO_MEDIA. If Aip protocol is not
> supported, function will
> + call NetLibDetectMedia() and return state directly.
> +
> + @param[in] ServiceHandle The handle where network service binding
> protocols are
> + installed on.
> + @param[in] Timeout The maximum number of 100ns units to wait
> when network
> + is connecting. Zero value means detect once and return
> + immediately.
> + @param[out] MediaState The pointer to the detected media state.
> +
> + @retval EFI_SUCCESS Media detection success.
> + @retval EFI_INVALID_PARAMETER ServiceHandle is not a valid network
> device handle or
> + MediaState pointer is NULL.
> + @retval EFI_DEVICE_ERROR A device error occurred.
> + @retval EFI_TIMEOUT Network is connecting but timeout.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +NetLibDetectMediaWaitTimeout (
> + IN EFI_HANDLE ServiceHandle,
> + IN UINT64 Timeout,
> + OUT EFI_STATUS *MediaState
> + );
> +
> +
> +/**
> Create an IPv4 device path node.
>
> The header type of IPv4 device path node is MESSAGING_DEVICE_PATH.
> The header subtype of IPv4 device path node is MSG_IPv4_DP.
> The length of the IPv4 device path node in bytes is 19.
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> index b8544b8..3030147 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> @@ -17,10 +17,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include <IndustryStandard/SmBios.h>
>
> #include <Protocol/DriverBinding.h>
> #include <Protocol/ServiceBinding.h>
> #include <Protocol/SimpleNetwork.h>
> +#include <Protocol/AdapterInformation.h>
> #include <Protocol/ManagedNetwork.h>
> #include <Protocol/Ip4Config2.h>
> #include <Protocol/ComponentName.h>
> #include <Protocol/ComponentName2.h>
>
> @@ -2501,10 +2502,169 @@ Exit:
>
> return Status;
> }
>
> /**
> +
> + Detect media state for a network device. This routine will wait for a period
> of time at
> + a specified checking interval when a certain network is under connecting
> until connection
> + process finishes or timeout. If Aip protocol is supported by low layer
> drivers, three kinds
> + of media states can be detected: EFI_SUCCESS, EFI_NOT_READY and
> EFI_NO_MEDIA, represents
> + connected state, connecting state and no media state respectively. When
> function detects
> + the current state is EFI_NOT_READY, it will loop to wait for next time's
> check until state
> + turns to be EFI_SUCCESS or EFI_NO_MEDIA. If Aip protocol is not
> supported, function will
> + call NetLibDetectMedia() and return state directly.
> +
> + @param[in] ServiceHandle The handle where network service binding
> protocols are
> + installed on.
> + @param[in] Timeout The maximum number of 100ns units to wait
> when network
> + is connecting. Zero value means detect once and return
> + immediately.
> + @param[out] MediaState The pointer to the detected media state.
> +
> + @retval EFI_SUCCESS Media detection success.
> + @retval EFI_INVALID_PARAMETER ServiceHandle is not a valid network
> device handle or
> + MediaState pointer is NULL.
> + @retval EFI_DEVICE_ERROR A device error occurred.
> + @retval EFI_TIMEOUT Network is connecting but timeout.
> +
> +**/
> +
> +EFI_STATUS
> +EFIAPI
> +NetLibDetectMediaWaitTimeout (
> + IN EFI_HANDLE ServiceHandle,
> + IN UINT64 Timeout,
> + OUT EFI_STATUS *MediaState
> + )
> +{
> + EFI_STATUS Status;
> + EFI_HANDLE SnpHandle;
> + EFI_SIMPLE_NETWORK_PROTOCOL *Snp;
> + EFI_ADAPTER_INFORMATION_PROTOCOL *Aip;
> + EFI_ADAPTER_INFO_MEDIA_STATE *MediaInfo;
> + BOOLEAN MediaPresent;
> + UINTN DataSize;
> + EFI_STATUS TimerStatus;
> + EFI_EVENT Timer;
> + UINT64 TimeRemained;
> +
> + ASSERT (MediaState != NULL);
> + if (MediaState == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> + *MediaState = EFI_NO_MEDIA;
> +
> + //
> + // Get SNP handle
> + //
> + Snp = NULL;
> + SnpHandle = NetLibGetSnpHandle (ServiceHandle, &Snp);
> + if (SnpHandle == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Status = gBS->HandleProtocol (
> + SnpHandle,
> + &gEfiAdapterInformationProtocolGuid,
> + (VOID *) &Aip
> + );
> + if (EFI_ERROR (Status)) {
> +
> + MediaPresent = TRUE;
> + Status = NetLibDetectMedia (ServiceHandle, &MediaPresent);
> + if (!EFI_ERROR (Status)) {
> + if (MediaPresent == TRUE) {
> + *MediaState = EFI_SUCCESS;
> + } else {
> + *MediaState = EFI_NO_MEDIA;
> + }
> + }
> +
> + //
> + // NetLibDetectMedia doesn't support EFI_NOT_READY status, return
> now!
> + //
> + return Status;
> + }
> +
> + Status = Aip->GetInformation (
> + Aip,
> + &gEfiAdapterInfoMediaStateGuid,
> + (VOID **) &MediaInfo,
> + &DataSize
> + );
> + if (!EFI_ERROR (Status)) {
> +
> + *MediaState = MediaInfo->MediaState;
> + if (*MediaState != EFI_NOT_READY) {
> +
> + FreePool (MediaInfo);
> + return EFI_SUCCESS;
> + }
> + }
> +
> + if (MediaInfo != NULL) {
> + FreePool (MediaInfo);
> + }
> +
> + if (Timeout < MEDIA_STATE_DETECT_TIME_INTERVAL) {
> + return Status;
> + }
> +
> + //
> + // Loop to check media state
> + //
> +
> + Timer = NULL;
> + TimeRemained = Timeout;
> + Status = gBS->CreateEvent (EVT_TIMER, TPL_CALLBACK, NULL, NULL,
> &Timer);
> + if (EFI_ERROR (Status)) {
> + return EFI_DEVICE_ERROR;
> + }
> +
> + do {
> + Status = gBS->SetTimer (
> + Timer,
> + TimerRelative,
> + MEDIA_STATE_DETECT_TIME_INTERVAL
> + );
> + if (EFI_ERROR (Status)) {
> + gBS->CloseEvent(Timer);
> + return EFI_DEVICE_ERROR;
> + }
> +
> + do {
> + TimerStatus = gBS->CheckEvent (Timer);
> + if (!EFI_ERROR (TimerStatus)) {
> +
> + TimeRemained -= MEDIA_STATE_DETECT_TIME_INTERVAL;
> + Status = Aip->GetInformation (
> + Aip,
> + &gEfiAdapterInfoMediaStateGuid,
> + (VOID **) &MediaInfo,
> + &DataSize
> + );
> + if (!EFI_ERROR (Status)) {
> + *MediaState = MediaInfo->MediaState;
> + }
> +
> + if (MediaInfo != NULL) {
> + FreePool (MediaInfo);
> + }
> + }
> + } while (TimerStatus == EFI_NOT_READY);
> + } while (*MediaState == EFI_NOT_READY && TimeRemained >=
> MEDIA_STATE_DETECT_TIME_INTERVAL);
> +
> + gBS->CloseEvent(Timer);
> + if (*MediaState == EFI_NOT_READY && TimeRemained <
> MEDIA_STATE_DETECT_TIME_INTERVAL) {
> + return EFI_TIMEOUT;
> + } else {
> + return EFI_SUCCESS;
> + }
> +}
> +
> +/**
> Check the default address used by the IPv4 driver is static or dynamic
> (acquired
> from DHCP).
>
> If the controller handle does not have the EFI_IP4_CONFIG2_PROTOCOL
> installed, the
> default address is static. If failed to get the policy from Ip4 Config2 Protocol,
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> index 1ff3a4f..ad0727c 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> @@ -52,14 +52,16 @@
>
>
> [Guids]
> gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ##
> SystemTable
> gEfiSmbios3TableGuid ## SOMETIMES_CONSUMES ##
> SystemTable
> + gEfiAdapterInfoMediaStateGuid ## SOMETIMES_CONSUMES
>
>
> [Protocols]
> gEfiSimpleNetworkProtocolGuid ## SOMETIMES_CONSUMES
> gEfiManagedNetworkProtocolGuid ## SOMETIMES_CONSUMES
> gEfiManagedNetworkServiceBindingProtocolGuid ##
> SOMETIMES_CONSUMES
> gEfiIp4Config2ProtocolGuid ## SOMETIMES_CONSUMES
> gEfiComponentNameProtocolGuid ## SOMETIMES_CONSUMES
> gEfiComponentName2ProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiAdapterInformationProtocolGuid ## SOMETIMES_CONSUMES
> \ No newline at end of file
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 2+ messages in thread