public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Jin, Eric" <eric.jin@intel.com>
To: Supreeth Venkatesh <supreeth.venkatesh@arm.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct Enable parameter in ReceiveFilters test
Date: Fri, 18 Jan 2019 05:13:04 +0000	[thread overview]
Message-ID: <DA72DC7456565B47808A57108259571F6373A5BC@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <6839a47d737c7dcd25e990beea7566bca3ac19c7.camel@arm.com>

Supreeth,

Yes, the MAC here is the purpose for test data only.
I will add the comment to clarify it when I push the code.

Best Regards
Eric

-----Original Message-----
From: Supreeth Venkatesh <supreeth.venkatesh@arm.com> 
Sent: Friday, January 18, 2019 5:01 AM
To: Jin, Eric <eric.jin@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2-test][Patch] uefi-sct/SctPkg:Correct Enable parameter in ReceiveFilters test


MAC address is hardcoded below which needs a comment or clarification to say it is test data (if it really is).
Otherwise, its good to go.

On Tue, 2018-11-27 at 14:02 +0800, Eric Jin wrote:
> The patch is applied to the EFI part.
> 
> Fix bug - set EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST bit in Enable 
> parameter to make sure Multicast is enabled.
> Add one checkpoint when MCastFilterCount is zero
> 
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@intel.com>
Acked-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>

> ---
>  .../EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.c |   4 +-
>  .../EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid.h |   7 +-
>  .../BlackBoxTest/SimpleNetworkBBTestConformance.c  | 198
> ++++++++++++---------
>  3 files changed, 119 insertions(+), 90 deletions(-)
> 
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid
> .c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid
> .c
> index 6ea6c4c..7234323 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid
> .c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid
> .c
> @@ -1,7 +1,7 @@
>  /** @file
>  
>    Copyright 2006 - 2016 Unified EFI, Inc.<BR>
> -  Copyright (c) 2010 - 2016, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2010 - 2018, Intel Corporation. All rights
> reserved.<BR>
>  
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of 
> the BSD License @@ -112,6 +112,8 @@ EFI_GUID
> gSimpleNetworkBBTestConformanceAssertionGuid041 = 
> EFI_TEST_SIMPLENETWOR
>  
>  EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid042 = 
> EFI_TEST_SIMPLENETWORKBBTESTCONFORMANCE_ASSERTION_042_GUID;
>  
> +EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid043 =
> EFI_TEST_SIMPLENETWORKBBTESTCONFORMANCE_ASSERTION_043_GUID;
> +
>  EFI_GUID gSimpleNetworkBBTestFunctionAssertionGuid001 = 
> EFI_TEST_SIMPLENETWORKBBTESTFUNCTION_ASSERTION_001_GUID;
>  
>  EFI_GUID gSimpleNetworkBBTestFunctionAssertionGuid002 = 
> EFI_TEST_SIMPLENETWORKBBTESTFUNCTION_ASSERTION_002_GUID;
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid
> .h b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid
> .h
> index 281d893..bf909d1 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid
> .h
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Guid
> .h
> @@ -1,7 +1,7 @@
>  /** @file
>  
>    Copyright 2006 - 2016 Unified EFI, Inc.<BR>
> -  Copyright (c) 2010 - 2016, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2010 - 2018, Intel Corporation. All rights
> reserved.<BR>
>  
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of 
> the BSD License @@ -235,6 +235,11 @@ extern EFI_GUID 
> gSimpleNetworkBBTestConformanceAssertionGuid041;
>  
>  extern EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid042;
>  
> +#define EFI_TEST_SIMPLENETWORKBBTESTCONFORMANCE_ASSERTION_043_GUID \ 
> +{ 0x8cec0b86, 0x7773, 0x4d3c, {0x84, 0x13, 0x26, 0x37, 0xfb, 0xd0,
> 0x8e, 0x1b }}
> +
> +extern EFI_GUID gSimpleNetworkBBTestConformanceAssertionGuid043;
> +
>  #define EFI_TEST_SIMPLENETWORKBBTESTFUNCTION_ASSERTION_001_GUID \  { 
> 0xf58651fe, 0x0538, 0x4407, {0x88, 0xe0, 0x88, 0xb8, 0xda, 0x18, 0x38, 
> 0x3a }}
>  
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Simp
> leNetworkBBTestConformance.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Simp
> leNetworkBBTestConformance.c
> index ccbbad0..03a0d04 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Simp
> leNetworkBBTestConformance.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Simp
> leNetworkBBTestConformance.c
> @@ -1,7 +1,7 @@
>  /** @file
>  
>    Copyright 2006 - 2016 Unified EFI, Inc.<BR>
> -  Copyright (c) 2010 - 2016, Intel Corporation. All rights 
> reserved.<BR>
> +  Copyright (c) 2010 - 2018, Intel Corporation. All rights
> reserved.<BR>
>  
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of 
> the BSD License @@ -581,10 +581,9 @@ 
> BBTestReceiveFilterConformanceTest (  {
>    EFI_STANDARD_TEST_LIBRARY_PROTOCOL    *StandardLib;
>    EFI_STATUS                            Status;
> -  EFI_STATUS                            StatusBuf[5];
> -  EFI_TEST_ASSERTION                    AssertionType[5];
> +  EFI_TEST_ASSERTION                    AssertionType;
>    EFI_SIMPLE_NETWORK_PROTOCOL           *SnpInterface;
> -  EFI_SIMPLE_NETWORK_STATE              State1, State2;
> +  EFI_MAC_ADDRESS                       MAC;
>  
>    //
>    // Get the Standard Library Interface @@ -608,16 +607,14 @@ 
> BBTestReceiveFilterConformanceTest (
>    // Check whether the state of network interface is 
> EfiSimpleNetworkStopped.
>    // If not, change the state to EfiSimpleNetworkStopped.
>    //
> -  State1 = SnpInterface->Mode->State;
> -  if (State1 == EfiSimpleNetworkInitialized) {
> +  if (SnpInterface->Mode->State == EfiSimpleNetworkInitialized) {
>      Status = SnpInterface->Shutdown (SnpInterface);
>      if (EFI_ERROR(Status)) {
>        return Status;
>      }
>    }
>  
> -  State2 = SnpInterface->Mode->State;
> -  if (State2 == EfiSimpleNetworkStarted) {
> +  if (SnpInterface->Mode->State == EfiSimpleNetworkStarted) {
>      Status = SnpInterface->Stop (SnpInterface);
>      if (EFI_ERROR(Status)) {
>        return Status;
> @@ -628,13 +625,24 @@ BBTestReceiveFilterConformanceTest (
>    // Assertion Point 5.6.2.1
>    // Call ReceiveFilters() function if network interface not start.
>    //
> -  StatusBuf[0] = SnpInterface->ReceiveFilters (SnpInterface, 0, 0, 
> FALSE, 0, NULL);
> -  if ((StatusBuf[0] == EFI_NOT_STARTED) && (SnpInterface->Mode-
> >State == EfiSimpleNetworkStopped)) {
> -    AssertionType[0] = EFI_TEST_ASSERTION_PASSED;
> +  Status = SnpInterface->ReceiveFilters (SnpInterface, 0, 0, FALSE,
> 0, NULL);
> +  if ((Status == EFI_NOT_STARTED) && (SnpInterface->Mode->State ==
> EfiSimpleNetworkStopped)) {
> +    AssertionType = EFI_TEST_ASSERTION_PASSED;
>    } else {
> -    AssertionType[0] = EFI_TEST_ASSERTION_FAILED;
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
>    }
>  
> +  StandardLib->RecordAssertion (
> +                 StandardLib,
> +                 AssertionType,
> +                 gSimpleNetworkBBTestConformanceAssertionGuid006,
> +                 L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() when network interface not start.",
> +                 L"%a:%d:Status - %r",
> +                 __FILE__,
> +                 (UINTN)__LINE__,
> +                 Status
> +                 );
> +
>    //
>    // Assertion Point 5.6.2.2
>    // Call ReceiveFilters() function if network interface not 
> initialized.
> @@ -644,13 +652,24 @@ BBTestReceiveFilterConformanceTest (
>      return Status;
>    }
>  
> -  StatusBuf[1] = SnpInterface->ReceiveFilters (SnpInterface, 0, 0, 
> FALSE, 0, NULL);
> -  if (StatusBuf[1] == EFI_DEVICE_ERROR) {
> -    AssertionType[1] = EFI_TEST_ASSERTION_PASSED;
> +  Status = SnpInterface->ReceiveFilters (SnpInterface, 0, 0, FALSE,
> 0, NULL);
> +  if (Status == EFI_DEVICE_ERROR) {
> +    AssertionType = EFI_TEST_ASSERTION_PASSED;
>    } else {
> -    AssertionType[1] = EFI_TEST_ASSERTION_FAILED;
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
>    }
> -
> +
> +  StandardLib->RecordAssertion (
> +                 StandardLib,
> +                 AssertionType,
> +                 gSimpleNetworkBBTestConformanceAssertionGuid007,
> +                 L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() when network interface not initialized.",
> +                 L"%a:%d:Status - %r",
> +                 __FILE__,
> +                 (UINTN)__LINE__,
> +                 Status
> +                 );
> +
>    //
>    // Assertion Point 5.6.2.3
>    // Call ReceiveFilters() function with invalid parameters.
> @@ -663,93 +682,96 @@ BBTestReceiveFilterConformanceTest (
>    //
>    //  Call ReceiveFilters with invalide Enable
>    //
> -  StatusBuf[2] = SnpInterface->ReceiveFilters (SnpInterface, 
> ~(SnpInterface->Mode->ReceiveFilterMask), 0, FALSE, 0, NULL);
> -  if (StatusBuf[2] == EFI_INVALID_PARAMETER) {
> -    AssertionType[2] = EFI_TEST_ASSERTION_PASSED;
> -  } else {
> -    AssertionType[2] = EFI_TEST_ASSERTION_FAILED;
> -  }
> -
> -  //
> -  //  Call ReceiveFilters with invalide MCastFilterCnt
> -  //
> -  StatusBuf[3] = SnpInterface->ReceiveFilters (SnpInterface, 0, 0, 
> FALSE, SnpInterface->Mode->MaxMCastFilterCount + 1, NULL);
> -  if (StatusBuf[3] == EFI_INVALID_PARAMETER) {
> -    AssertionType[3] = EFI_TEST_ASSERTION_PASSED;
> -  } else {
> -    AssertionType[3] = EFI_TEST_ASSERTION_FAILED;
> -  }
> -
> -  //
> -  //  Call ReceiveFilters with MCastFilterCnt not match MCastFilter
> -  //
> -  StatusBuf[4] = SnpInterface->ReceiveFilters (SnpInterface, 0, 0, 
> FALSE, 1, NULL);
> -  if (StatusBuf[4] == EFI_INVALID_PARAMETER) {
> -    AssertionType[4] = EFI_TEST_ASSERTION_PASSED;
> +  Status = SnpInterface->ReceiveFilters (SnpInterface,
> ~(SnpInterface->Mode->ReceiveFilterMask), 0, FALSE, 0, NULL);
> +  if (Status == EFI_INVALID_PARAMETER) {
> +    AssertionType = EFI_TEST_ASSERTION_PASSED;
>    } else {
> -    AssertionType[4] = EFI_TEST_ASSERTION_FAILED;
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
>    }
>  
> -
> -  StandardLib->RecordAssertion (
> -                 StandardLib,
> -                 AssertionType[0],
> -                 gSimpleNetworkBBTestConformanceAssertionGuid006,
> -                 L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() when network interface not start.",
> -                 L"%a:%d:Status - %r",
> -                 __FILE__,
> -                 (UINTN)__LINE__,
> -                 StatusBuf[0]
> -                 );
> -
> -  StandardLib->RecordAssertion (
> -                 StandardLib,
> -                 AssertionType[1],
> -                 gSimpleNetworkBBTestConformanceAssertionGuid007,
> -                 L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() when network interface not initialized.",
> -                 L"%a:%d:Status - %r",
> -                 __FILE__,
> -                 (UINTN)__LINE__,
> -                 StatusBuf[1]
> -                 );
> -  
>    StandardLib->RecordAssertion (
>                   StandardLib,
> -                 AssertionType[2],
> +                 AssertionType,
>                   gSimpleNetworkBBTestConformanceAssertionGuid008,
>                   L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters - 
> Invoke ReceiveFilters() with invalid Enable.",
>                   L"%a:%d:Status - %r",
>                   __FILE__,
>                   (UINTN)__LINE__,
> -                 StatusBuf[2]
> +                 Status
>                   );
>  
> -  StandardLib->RecordAssertion (
> -                 StandardLib,
> -                 AssertionType[3],
> -                 gSimpleNetworkBBTestConformanceAssertionGuid009,
> -                 L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() with invalid MCastFilterCnt.",
> -                 L"%a:%d:Status - %r",
> -                 __FILE__,
> -                 (UINTN)__LINE__,
> -                 StatusBuf[3]
> -                 );
> +  //
> +  //  Call ReceiveFilters with invalide MCastFilterCnt  //  if 
> + ((SnpInterface->Mode->ReceiveFilterMask &
> EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST) != 0) {
> +    SctSetMem (&MAC, sizeof (MAC), 0x00);
> +    MAC.Addr[0] = 0x01;
> +    MAC.Addr[1] = 0x00;
> +    MAC.Addr[2] = 0x5e;
> +    MAC.Addr[3] = 0x00;
> +    MAC.Addr[4] = 0x00;
> +    MAC.Addr[5] = 0x02;
Why is the MAC hardcoded. Clarification?

> +
> +    Status = SnpInterface->ReceiveFilters (SnpInterface,
> EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST, 0, FALSE, SnpInterface->Mode-
> >MaxMCastFilterCount + 1, &MAC);
> +    if (Status == EFI_INVALID_PARAMETER) {
> +      AssertionType = EFI_TEST_ASSERTION_PASSED;
> +    } else {
> +      AssertionType = EFI_TEST_ASSERTION_FAILED;
> +    }
>  
> -  StandardLib->RecordAssertion (
> -                 StandardLib,
> -                 AssertionType[4],
> -                 gSimpleNetworkBBTestConformanceAssertionGuid010,
> -                 L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() with MCastFilterCnt not match MCastFilter.",
> -                 L"%a:%d:Status - %r",
> -                 __FILE__,
> -                 (UINTN)__LINE__,
> -                 StatusBuf[4]
> -                 );
> +    StandardLib->RecordAssertion (
> +                   StandardLib,
> +                   AssertionType,
> +                   gSimpleNetworkBBTestConformanceAssertionGuid009,
> +                   L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() with invalid MCastFilterCnt is greater than
> Snp->Mode->MaxMCastFilterCount.",
> +                   L"%a:%d:Status - %r",
> +                   __FILE__,
> +                   (UINTN)__LINE__,
> +                   Status
> +                   );
> +
> +    Status = SnpInterface->ReceiveFilters (SnpInterface,
> EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST, 0, FALSE, 0, &MAC);
> +    if (Status == EFI_INVALID_PARAMETER) {
> +      AssertionType = EFI_TEST_ASSERTION_PASSED;
> +    } else {
> +      AssertionType = EFI_TEST_ASSERTION_FAILED;
> +    }
> +
> +    StandardLib->RecordAssertion (
> +                   StandardLib,
> +                   AssertionType,
> +                   gSimpleNetworkBBTestConformanceAssertionGuid043,
> +                   L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() with invalid MCastFilterCnt is 0.",
> +                   L"%a:%d:Status - %r",
> +                   __FILE__,
> +                   (UINTN)__LINE__,
> +                   Status
> +                   );
> +
> +    Status = SnpInterface->ReceiveFilters (SnpInterface,
> EFI_SIMPLE_NETWORK_RECEIVE_MULTICAST, 0, FALSE, 1, NULL);
> +    if (Status == EFI_INVALID_PARAMETER) {
> +      AssertionType = EFI_TEST_ASSERTION_PASSED;
> +    } else {
> +      AssertionType = EFI_TEST_ASSERTION_FAILED;
> +    }
> +
> +    StandardLib->RecordAssertion (
> +                   StandardLib,
> +                   AssertionType,
> +                   gSimpleNetworkBBTestConformanceAssertionGuid010,
> +                   L"EFI_SIMPLE_NETWORK_PROTOCOL.ReceiveFilters -
> Invoke ReceiveFilters() with MCastFilterCnt not match MCastFilter.",
> +                   L"%a:%d:Status - %r",
> +                   __FILE__,
> +                   (UINTN)__LINE__,
> +                   Status
> +                   );
> +
> +  }
>  
>    //
>    // Restore SNP State
>    //
> -  if (State1 == EfiSimpleNetworkStopped) {
> +  if (SnpInterface->Mode->State == EfiSimpleNetworkStopped) {
>      Status = SnpInterface->Shutdown (SnpInterface);
>      if (EFI_ERROR(Status)) {
>        return Status;
> @@ -760,7 +782,7 @@ BBTestReceiveFilterConformanceTest (
>        return Status;
>      }
>    }
> -
> +
>    return EFI_SUCCESS;
>  }
>  


  reply	other threads:[~2019-01-18  5:13 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-27  6:02 [edk2-test][Patch] uefi-sct/SctPkg:Correct Enable parameter in ReceiveFilters test Eric Jin
2019-01-17 21:00 ` Supreeth Venkatesh
2019-01-18  5:13   ` Jin, Eric [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-11-27  5:57 Eric Jin
2019-01-17 21:02 ` Supreeth Venkatesh

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=DA72DC7456565B47808A57108259571F6373A5BC@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