diff mbox

[NEXT,1/2] net: add external loopback test in ethtool self test

Message ID 1309243247-15950-2-git-send-email-amit.salecha@qlogic.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

amit salecha June 28, 2011, 6:40 a.m. UTC
From: Amit Kumar Salecha <amit.salecha@qlogic.com>

External loopback test can be performed by application without any driver
support on normal Ethernet cards.
But on CNA devices, where multiple functions share same physical port.
Here internal loopback test and external loopback test can be initiated by

Comments

Ben Hutchings June 29, 2011, 6:28 p.m. UTC | #1
On Mon, 2011-06-27 at 23:40 -0700, amit.salecha@qlogic.com wrote:
> From: Amit Kumar Salecha <amit.salecha@qlogic.com>
> 
> External loopback test can be performed by application without any driver
> support on normal Ethernet cards.
> But on CNA devices, where multiple functions share same physical port.
> Here internal loopback test and external loopback test can be initiated by
> different function at same time. To co exist all functions, firmware need
> to regulate what test can be run by which function. So before performing external
> loopback test, command need to send to firmware, which will quiescent other functions.
> 
> User may not want to run external loopback test always. As special cable need to be
> connected for this test.
> 
> So adding explicit flag in ethtool self test, which will specify interface
> to perform external loopback test.

The trouble with adding flags to enum ethtool_test_flags is that there
is really no general way to tell whether the driver understood the flag.

I think you need to add a second flag which the driver sets to confirm
that it *did* use external loopback.

> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> ---
>  include/linux/ethtool.h |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 048d0fa..c2ba287 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -310,9 +310,18 @@ struct ethtool_sset_info {
>  				   __u32's, etc. */
>  };
>  
> +/*
> + * Flags definition of ethtool_test
> + *
> + * ETH_TEST_FL_OFFLINE:  online / offline
> + * ETH_TEST_FL_FAILED: test passed / failed
> + * ETH_TEST_FL_EXTERNAL_LB: perform external loopback test
> + */
> +

Replacing the inline comments with a block comment is fine, but please
use kernel-doc format.

Ben.

>  enum ethtool_test_flags {
> -	ETH_TEST_FL_OFFLINE	= (1 << 0),	/* online / offline */
> -	ETH_TEST_FL_FAILED	= (1 << 1),	/* test passed / failed */
> +	ETH_TEST_FL_OFFLINE	= (1 << 0),
> +	ETH_TEST_FL_FAILED	= (1 << 1),
> +	ETH_TEST_FL_EXTERNAL_LB	= (1 << 2),
>  };
>  
>  /* for requesting NIC test and getting results*/
amit salecha June 30, 2011, 5:26 a.m. UTC | #2
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]

>

> Subject: Re: [PATCH NEXT 1/2] net: add external loopback test in

> ethtool self test

> 

> >

> > So adding explicit flag in ethtool self test, which will specify

> interface

> > to perform external loopback test.

> 

> The trouble with adding flags to enum ethtool_test_flags is that there

> is really no general way to tell whether the driver understood the

> flag.

> 

> I think you need to add a second flag which the driver sets to confirm

> that it *did* use external loopback.

>


If I understood correctly:
You are concern about drives which doesn't support external loopback test.
In this case application doesn't have no general way to know, whether driver has
performed external loopback test.
Though the test case will be mention in test set. (return by .get_strings)

I will add it and send revised patch.

> > +/*

> > + * Flags definition of ethtool_test

> > + *

> > + * ETH_TEST_FL_OFFLINE:  online / offline

> > + * ETH_TEST_FL_FAILED: test passed / failed

> > + * ETH_TEST_FL_EXTERNAL_LB: perform external loopback test

> > + */

> > +

> 

> Replacing the inline comments with a block comment is fine, but please

> use kernel-doc format.

> 

I will fix it.

-Amit
diff mbox

Patch

different function at same time. To co exist all functions, firmware need
to regulate what test can be run by which function. So before performing external
loopback test, command need to send to firmware, which will quiescent other functions.

User may not want to run external loopback test always. As special cable need to be
connected for this test.

So adding explicit flag in ethtool self test, which will specify interface
to perform external loopback test.

Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 include/linux/ethtool.h |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 048d0fa..c2ba287 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -310,9 +310,18 @@  struct ethtool_sset_info {
 				   __u32's, etc. */
 };
 
+/*
+ * Flags definition of ethtool_test
+ *
+ * ETH_TEST_FL_OFFLINE:  online / offline
+ * ETH_TEST_FL_FAILED: test passed / failed
+ * ETH_TEST_FL_EXTERNAL_LB: perform external loopback test
+ */
+
 enum ethtool_test_flags {
-	ETH_TEST_FL_OFFLINE	= (1 << 0),	/* online / offline */
-	ETH_TEST_FL_FAILED	= (1 << 1),	/* test passed / failed */
+	ETH_TEST_FL_OFFLINE	= (1 << 0),
+	ETH_TEST_FL_FAILED	= (1 << 1),
+	ETH_TEST_FL_EXTERNAL_LB	= (1 << 2),
 };
 
 /* for requesting NIC test and getting results*/