diff mbox series

[net-next,1/3] selftests: mlxsw: Add ethtool_lib.sh

Message ID 20190610084045.6029-2-idosch@idosch.org
State Changes Requested
Delegated to: David Miller
Headers show
Series mlxsw: Add speed and auto-negotiation test | expand

Commit Message

Ido Schimmel June 10, 2019, 8:40 a.m. UTC 
From: Amit Cohen <amitc@mellanox.com>

Functions:
1. ethtool_set:
	params: cmd
	The function runs ethtool by cmd (ethtool -s cmd) and checks if there
	 was an error in configuration

2. speeds_get:
	params: dev, with_mode (0 or 1)
	return value: Array of supported link modes with/without mode.

	* Example 1:
	speeds_get swp1 0
	return: 1000 10000 40000
	* Example 2:
	speeds_get swp1 1
	return: 1000baseKX/Full 10000baseKR/Full 40000baseCR4/Full 40000baseSR4/Full

3. common_speeds_get:
	params: dev1, dev2, with_mode (0 or 1)
	return value: Array of common speeds of dev1 and dev2.

	* Example:
	common_speeds_get swp1 swp2 0
	return: 1000 10000
	(assume that swp1 supports 1000 10000 40000 and swp2 supports 1000 10000)

Signed-off-by: Amit Cohen <amitc@mellanox.com>
Reviewed-by: Petr Machata <petrm@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
 .../selftests/net/forwarding/ethtool_lib.sh   | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100755 tools/testing/selftests/net/forwarding/ethtool_lib.sh

Comments

Andrew Lunn June 10, 2019, 1:35 p.m. UTC | #1 
On Mon, Jun 10, 2019 at 11:40:43AM +0300, Ido Schimmel wrote:
> From: Amit Cohen <amitc@mellanox.com>
> +declare -A speed_values
> +
> +speed_values=(	[10baseT/Half]=0x001
> +		[10baseT/Full]=0x002
> +		[100baseT/Half]=0x004
> +		[100baseT/Full]=0x008
> +		[1000baseT/Half]=0x010
> +		[1000baseT/Full]=0x020

Hi Ido, Amit

100BaseT1 and 1000BaseT1 were added recently.

	  Andrew
Ido Schimmel June 10, 2019, 1:56 p.m. UTC | #2 
On Mon, Jun 10, 2019 at 03:35:38PM +0200, Andrew Lunn wrote:
> On Mon, Jun 10, 2019 at 11:40:43AM +0300, Ido Schimmel wrote:
> > From: Amit Cohen <amitc@mellanox.com>
> > +declare -A speed_values
> > +
> > +speed_values=(	[10baseT/Half]=0x001
> > +		[10baseT/Full]=0x002
> > +		[100baseT/Half]=0x004
> > +		[100baseT/Full]=0x008
> > +		[1000baseT/Half]=0x010
> > +		[1000baseT/Full]=0x020
> 
> Hi Ido, Amit
> 
> 100BaseT1 and 1000BaseT1 were added recently.

Hi Andrew,

Didn't see them in the man page, so didn't include them. I now see your
patches are in the queue. Will add these speeds in v2.

Thanks
Andrew Lunn June 10, 2019, 1:59 p.m. UTC | #3 
> +speeds_get()
> +{
> +	local dev=$1; shift
> +	local with_mode=$1; shift
> +
> +	local speeds_str=$(ethtool "$dev" | \
> +		# Snip everything before the link modes section.
> +		sed -n '/Supported link modes:/,$p' | \
> +		# Quit processing the rest at the start of the next section.
> +		# When checking, skip the header of this section (hence the 2,).
> +		sed -n '2,${/^[\t][^ \t]/q};p' | \
> +		# Drop the section header of the current section.
> +		cut -d':' -f2)

ethtool gives you two lists of link modes:

$ sudo ethtool eth17
Settings for eth17:
         Supported ports: [ TP ]
         Supported link modes:   10baseT/Half 10baseT/Full 
                                 100baseT/Half 100baseT/Full 
                                 1000baseT/Full 
         Supported pause frame use: No
         Supports auto-negotiation: Yes
         Supported FEC modes: Not reported
         Advertised link modes:  10baseT/Half 10baseT/Full 
                                 100baseT/Half 100baseT/Full 
                                 1000baseT/Full

and if auto-neg has completed, there is potentially a third list, what
the peer is advertising.

Since this test is all about auto-neg, you should be using Advertised
link modes, not Supported link modes. There can be supported link
modes which you cannot advertise.

   Andrew
Ido Schimmel June 10, 2019, 2:31 p.m. UTC | #4 
On Mon, Jun 10, 2019 at 03:59:14PM +0200, Andrew Lunn wrote:
> > +speeds_get()
> > +{
> > +	local dev=$1; shift
> > +	local with_mode=$1; shift
> > +
> > +	local speeds_str=$(ethtool "$dev" | \
> > +		# Snip everything before the link modes section.
> > +		sed -n '/Supported link modes:/,$p' | \
> > +		# Quit processing the rest at the start of the next section.
> > +		# When checking, skip the header of this section (hence the 2,).
> > +		sed -n '2,${/^[\t][^ \t]/q};p' | \
> > +		# Drop the section header of the current section.
> > +		cut -d':' -f2)
> 
> ethtool gives you two lists of link modes:
> 
> $ sudo ethtool eth17
> Settings for eth17:
>          Supported ports: [ TP ]
>          Supported link modes:   10baseT/Half 10baseT/Full 
>                                  100baseT/Half 100baseT/Full 
>                                  1000baseT/Full 
>          Supported pause frame use: No
>          Supports auto-negotiation: Yes
>          Supported FEC modes: Not reported
>          Advertised link modes:  10baseT/Half 10baseT/Full 
>                                  100baseT/Half 100baseT/Full 
>                                  1000baseT/Full
> 
> and if auto-neg has completed, there is potentially a third list, what
> the peer is advertising.
> 
> Since this test is all about auto-neg, you should be using Advertised
> link modes, not Supported link modes. There can be supported link
> modes which you cannot advertise.

Andrew, are you suggestion to split speeds_get() into
supported_speeds_get() and advertised_speeds_get() and use each where
appropriate? Note that not all the tests are testing with autoneg on.
Andrew Lunn June 10, 2019, 2:51 p.m. UTC | #5 
> Andrew, are you suggestion to split speeds_get() into
> supported_speeds_get() and advertised_speeds_get() and use each where
> appropriate? Note that not all the tests are testing with autoneg on.

Hi Ido

Yes.

You should be able to force all speeds in supported speeds. But if you
try to auto-neg an speed which is not listed in advertised speeds when
all are enabled, i would expect an error.

    Andrew
Florian Fainelli June 10, 2019, 3:29 p.m. UTC | #6 
On 6/10/2019 6:56 AM, Ido Schimmel wrote:
> On Mon, Jun 10, 2019 at 03:35:38PM +0200, Andrew Lunn wrote:
>> On Mon, Jun 10, 2019 at 11:40:43AM +0300, Ido Schimmel wrote:
>>> From: Amit Cohen <amitc@mellanox.com>
>>> +declare -A speed_values
>>> +
>>> +speed_values=(	[10baseT/Half]=0x001
>>> +		[10baseT/Full]=0x002
>>> +		[100baseT/Half]=0x004
>>> +		[100baseT/Full]=0x008
>>> +		[1000baseT/Half]=0x010
>>> +		[1000baseT/Full]=0x020
>>
>> Hi Ido, Amit
>>
>> 100BaseT1 and 1000BaseT1 were added recently.
> 
> Hi Andrew,
> 
> Didn't see them in the man page, so didn't include them. I now see your
> patches are in the queue. Will add these speeds in v2.

Could we extract the values from include/uapi/linux/ethtool.h, that way
we would not have to have to update the selftest speed_values() array here?
Ido Schimmel June 11, 2019, 6:51 a.m. UTC | #7 
On Mon, Jun 10, 2019 at 08:29:54AM -0700, Florian Fainelli wrote:
> On 6/10/2019 6:56 AM, Ido Schimmel wrote:
> > On Mon, Jun 10, 2019 at 03:35:38PM +0200, Andrew Lunn wrote:
> >> On Mon, Jun 10, 2019 at 11:40:43AM +0300, Ido Schimmel wrote:
> >>> From: Amit Cohen <amitc@mellanox.com>
> >>> +declare -A speed_values
> >>> +
> >>> +speed_values=(	[10baseT/Half]=0x001
> >>> +		[10baseT/Full]=0x002
> >>> +		[100baseT/Half]=0x004
> >>> +		[100baseT/Full]=0x008
> >>> +		[1000baseT/Half]=0x010
> >>> +		[1000baseT/Full]=0x020
> >>
> >> Hi Ido, Amit
> >>
> >> 100BaseT1 and 1000BaseT1 were added recently.
> > 
> > Hi Andrew,
> > 
> > Didn't see them in the man page, so didn't include them. I now see your
> > patches are in the queue. Will add these speeds in v2.
> 
> Could we extract the values from include/uapi/linux/ethtool.h, that way
> we would not have to have to update the selftest speed_values() array here?

Hi Florian,

Sounds reasonable. Will try this out.
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/forwarding/ethtool_lib.sh b/tools/testing/selftests/net/forwarding/ethtool_lib.sh
new file mode 100755
index 000000000000..6dcbbd228047
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/ethtool_lib.sh
@@ -0,0 +1,91 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+declare -A speed_values
+
+speed_values=(	[10baseT/Half]=0x001
+		[10baseT/Full]=0x002
+		[100baseT/Half]=0x004
+		[100baseT/Full]=0x008
+		[1000baseT/Half]=0x010
+		[1000baseT/Full]=0x020
+		[1000baseKX/Full]=0x20000
+		[1000baseX/Full]=0x20000000000
+		[2500baseT/Full]=0x800000000000
+		[2500baseX/Full]=0x8000
+		[5000baseT/Full]=0x1000000000000
+		[10000baseT/Full]=0x1000
+		[10000baseKX4/Full]=0x40000
+		[10000baseKR/Full]=0x80000
+		[10000baseCR/Full]=0x40000000000
+		[10000baseSR/Full]=0x80000000000
+		[10000baseLR/Full]=0x100000000000
+		[10000baseLRM/Full]=0x200000000000
+		[10000baseER/Full]=0x400000000000
+		[20000baseMLD2/Full]=0x200000
+		[20000baseKR2/Full]=0x400000
+		[25000baseCR/Full]=0x80000000
+		[25000baseKR/Full]=0x100000000
+		[25000baseSR/Full]=0x200000000
+		[40000baseKR4/Full]=0x800000
+		[40000baseCR4/Full]=0x1000000
+		[40000baseSR4/Full]=0x2000000
+		[40000baseLR4/Full]=0x4000000
+		[50000baseCR2/Full]=0x400000000
+		[40000baseSR4/Full]=0x2000000
+		[40000baseLR4/Full]=0x4000000
+		[50000baseCR2/Full]=0x400000000
+		[50000baseKR2/Full]=0x800000000
+		[50000baseSR2/Full]=0x10000000000
+		[56000baseKR4/Full]=0x8000000
+		[56000baseCR4/Full]=0x10000000
+		[56000baseSR4/Full]=0x20000000
+		[56000baseLR4/Full]=0x40000000
+		[100000baseKR4/Full]=0x1000000000
+		[100000baseSR4/Full]=0x2000000000
+		[100000baseCR4/Full]=0x4000000000
+		[100000baseLR4_ER4/Full]=0x8000000000)
+
+ethtool_set()
+{
+	local cmd="$@"
+	local out=$(ethtool -s $cmd 2>&1 | wc -l)
+	check_err $out "error in configuration. $cmd"
+}
+
+speeds_get()
+{
+	local dev=$1; shift
+	local with_mode=$1; shift
+
+	local speeds_str=$(ethtool "$dev" | \
+		# Snip everything before the link modes section.
+		sed -n '/Supported link modes:/,$p' | \
+		# Quit processing the rest at the start of the next section.
+		# When checking, skip the header of this section (hence the 2,).
+		sed -n '2,${/^[\t][^ \t]/q};p' | \
+		# Drop the section header of the current section.
+		cut -d':' -f2)
+
+	local -a speeds_arr=($speeds_str)
+	if [[ $with_mode -eq 0 ]]; then
+		for ((i=0; i<${#speeds_arr[@]}; i++)); do
+			speeds_arr[$i]=${speeds_arr[$i]%base*}
+		done
+	fi
+	echo ${speeds_arr[@]}
+}
+
+common_speeds_get()
+{
+	dev1=$1; shift
+	dev2=$1; shift
+	with_mode=$1; shift
+
+	local -a dev1_speeds=($(speeds_get $dev1 $with_mode))
+	local -a dev2_speeds=($(speeds_get $dev2 $with_mode))
+
+	comm -12 \
+		<(printf '%s\n' "${dev1_speeds[@]}" | sort -u) \
+		<(printf '%s\n' "${dev2_speeds[@]}" | sort -u)
+}