[dts] [PATCH 0/4] ThunderX DTS initialization phase fixes

[dts] [PATCH 0/4] ThunderX DTS initialization phase fixes

[Radoslaw Biernacki]

This series fixes various bugs in DTS found during ThunderX testing session.

Radoslaw Biernacki (4):
  framework/crb: Fixing ThunderX ethernet controler detection
  framework: Fixing unnamed interface detection
  framework/dut: fixing mixed tab and space python indention
  framework/dut: Adding exception in case ports_map is empty

 framework/crb.py    | 11 ++-------
 framework/dut.py    | 71 +++++++++++++++++++++++++++--------------------------
 framework/tester.py |  5 +++-
 3 files changed, 42 insertions(+), 45 deletions(-)

-- 
2.7.4

[dts] [PATCH 1/4] framework/crb: Fixing ThunderX ethernet controler detection

[Radoslaw Biernacki]

Asking for link speed for ThunderX Ethernet controller is not reliable
since driver report error when the link is down. In fact we dont need
to ask for link speed as Ethernet controllers can be easily identified
by device name from lspci. The mapping will fuhrer filter out the PF
and VF interfaces which does not have the interface name assigned.

Fixes: 150716d93f5e ("framework crb: Appending only 10G devices for cavium")

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
---
 framework/crb.py | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/framework/crb.py b/framework/crb.py
index dd29a8b..36b1ffe 100644
--- a/framework/crb.py
+++ b/framework/crb.py
@@ -268,20 +268,13 @@ class Crb(object):
         Look for the NIC's information (PCI Id and card type).
         """
         out = self.send_expect(
-            "lspci -Dnn | grep -i eth", "# ", alt_session=True)
+            "lspci -Dnn | grep -i 'Ethernet controller'", "# ", alt_session=True)
         rexp = r"([\da-f]{4}:[\da-f]{2}:[\da-f]{2}.\d{1}) .*Eth.*?ernet .*?([\da-f]{4}:[\da-f]{4})"
         pattern = re.compile(rexp)
         match = pattern.findall(out)
         self.pci_devices_info = []
         for i in range(len(match)):
-            #check if device is cavium and check its linkspeed, append only if it is 10G
-            if "177d:" in match[i][1]:
-                linkspeed = "10000"
-                nic_linkspeed = self.send_command("cat /sys/bus/pci/devices/%s/net/*/speed" % match[i][0])
-                if nic_linkspeed == linkspeed:
-                    self.pci_devices_info.append((match[i][0], match[i][1]))
-            else:
-                self.pci_devices_info.append((match[i][0], match[i][1]))
+            self.pci_devices_info.append((match[i][0], match[i][1]))
 
     def pci_devices_information_uncached_freebsd(self):
         """
-- 
2.7.4

[dts] [PATCH 2/4] framework: Fixing unnamed interface detection

[Radoslaw Biernacki]

This patch fixes interface name detection broken in one of previous commits.
With introduction of mentioned commit, when interface has no name
get_interface_name() returns "N/A" instead of "No such file".

Fixes: f3e7b094176d ("fix bug that unbound device then run dts will show wrong interface")

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
---
 framework/dut.py    | 46 +++++++++++++++++++++++-----------------------
 framework/tester.py |  5 ++++-
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/framework/dut.py b/framework/dut.py
index 22ff0bb..c83377a 100644
--- a/framework/dut.py
+++ b/framework/dut.py
@@ -260,25 +260,29 @@ class Dut(Crb):
             pci_id = port['type']
             # get device driver
             driver = settings.get_nic_driver(pci_id)
-            if driver is not None:
-                # unbind device driver
-                addr_array = pci_bus.split(':')
-                domain_id = addr_array[0]
-                bus_id = addr_array[1]
-                devfun_id = addr_array[2]
+            if driver is None:
+                self.logger.info("NOT FOUND DRIVER FOR PORT (%s|%s)!!!" % (pci_bus, pci_id))
+                continue
+            # unbind device driver
+            addr_array = pci_bus.split(':')
+            domain_id = addr_array[0]
+            bus_id = addr_array[1]
+            devfun_id = addr_array[2]
 
-                port = GetNicObj(self, domain_id, bus_id, devfun_id)
+            port = GetNicObj(self, domain_id, bus_id, devfun_id)
 
-                self.send_expect('echo %s > /sys/bus/pci/devices/%s\:%s\:%s/driver/unbind'
-                                 % (pci_bus, domain_id, bus_id, devfun_id), '# ')
-                # bind to linux kernel driver
-                self.send_expect('modprobe %s' % driver, '# ')
-                self.send_expect('echo %s > /sys/bus/pci/drivers/%s/bind'
-                                 % (pci_bus, driver), '# ')
-                itf = port.get_interface_name()
-                self.send_expect("ifconfig %s up" % itf, "# ")
-            else:
-                self.logger.info("NOT FOUND DRIVER FOR PORT (%s|%s)!!!" % (pci_bus, pci_id))
+            self.send_expect('echo %s > /sys/bus/pci/devices/%s\:%s\:%s/driver/unbind'
+                             % (pci_bus, domain_id, bus_id, devfun_id), '# ')
+            # bind to linux kernel driver
+            self.send_expect('modprobe %s' % driver, '# ')
+            self.send_expect('echo %s > /sys/bus/pci/drivers/%s/bind'
+                             % (pci_bus, driver), '# ')
+            itf = port.get_interface_name()
+
+            if "N/A" in itf:
+                self.logger.warning("Cannot bring up the interface due missing inf name")
+                continue
+            self.send_expect("ifconfig %s up" % itf, "# ")
 
     def setup_memory(self, hugepages=-1):
         """
@@ -619,7 +623,7 @@ class Dut(Crb):
         for port_info in self.ports_info:
             port = port_info['port']
             intf = port.get_interface_name()
-            if "No such file" in intf:
+            if "N/A" in intf:
                 self.logger.info("DUT: [%s] %s" % (pci_bus, unknow_interface))
                 continue
             self.send_expect("ifconfig %s up" % intf, "# ")
@@ -717,15 +721,11 @@ class Dut(Crb):
 
             port = GetNicObj(self, domain_id, bus_id, devfun_id)
             intf = port.get_interface_name()
-            if "No such file" in intf:
+            if "N/A" in intf:
                 self.logger.info("DUT: [%s] %s" % (pci_bus, unknow_interface))
                 continue
 
             macaddr = port.get_mac_addr()
-            if "No such file" in intf:
-                self.logger.info("DUT: [%s] %s" % (pci_bus, unknow_interface))
-                continue
-
             numa = port.socket
             # store the port info to port mapping
             self.ports_info.append(
diff --git a/framework/tester.py b/framework/tester.py
index 1c854d7..b612cf1 100644
--- a/framework/tester.py
+++ b/framework/tester.py
@@ -237,6 +237,9 @@ class Tester(Crb):
                 addr_array = pci_bus.split(':')
                 port = GetNicObj(self, addr_array[0], addr_array[1], addr_array[2])
                 itf = port.get_interface_name()
+                if "N/A" in itf:
+                    self.logger.warning("Cannot bring up the interface due missing inf name")
+                    continue
                 self.enable_ipv6(itf)
                 self.send_expect("ifconfig %s up" % itf, "# ")
                 if port.get_interface2_name():
@@ -340,7 +343,7 @@ class Tester(Crb):
             port = GetNicObj(self, domain_id, bus_id, devfun_id)
             intf = port.get_interface_name()
 
-            if "No such file" in intf:
+            if "N/A" in intf:
                 self.logger.info("Tester: [%s %s] %s" % (pci_bus, pci_id,
                                                              "unknow_interface"))
                 continue
-- 
2.7.4

[dts] [PATCH 3/4] framework/dut: fixing mixed tab and space python indention

[Radoslaw Biernacki]

Fixes: bae359c3ae6c ("framework: checking link with IPv4 ping")

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
---
 framework/dut.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/framework/dut.py b/framework/dut.py
index c83377a..ee5608b 100644
--- a/framework/dut.py
+++ b/framework/dut.py
@@ -932,19 +932,19 @@ class Dut(Crb):
                 ipv6 = self.get_ipv6_address(dutPort)
                 if ipv6 == "Not connected":
                     if self.tester.ports_info[remotePort].has_key('ipv4'):
-			out = self.tester.send_ping(
-				dutPort, self.tester.ports_info[remotePort]['ipv4'],
-				self.get_mac_address(dutPort))
-		    else:
-                    	continue
-		else:
+                        out = self.tester.send_ping(
+                                dutPort, self.tester.ports_info[remotePort]['ipv4'],
+                                self.get_mac_address(dutPort))
+                    else:
+                        continue
+                else:
                     if getattr(self, 'send_ping6', None):
-                    	out = self.send_ping6(
-                        	dutPort, self.tester.ports_info[remotePort]['ipv6'],
-                        	self.get_mac_address(dutPort))
+                            out = self.send_ping6(
+                                dutPort, self.tester.ports_info[remotePort]['ipv6'],
+                                self.get_mac_address(dutPort))
                     else:
-                    	out = self.tester.send_ping6(
-				remotePort, ipv6, self.get_mac_address(dutPort))
+                            out = self.tester.send_ping6(
+                                remotePort, ipv6, self.get_mac_address(dutPort))
 
                 if ('64 bytes from' in out):
                     self.logger.info("PORT MAP: [dut %d: tester %d]" % (dutPort, remotePort))
-- 
2.7.4

[dts] [PATCH 4/4] framework/dut: Adding exception in case ports_map is empty

[Radoslaw Biernacki]

dut_prerequisites() warns with message that "ports_map cannot be
empty". If in fact it cannot be empty it should throw an exception.
Empty port map cause problems in most of the test anyway, so it is better to
crash hard than pushing on diging into the test to find the empty port map as
root cause.

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
---
 framework/dut.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/framework/dut.py b/framework/dut.py
index ee5608b..bf4dc18 100644
--- a/framework/dut.py
+++ b/framework/dut.py
@@ -212,7 +212,8 @@ class Dut(Crb):
         for port_info in self.ports_info:
             self.logger.info(port_info)
         if self.ports_map is None or len(self.ports_map) == 0:
-            self.logger.warning("ports_map should not be empty, please check all links")
+            self.logger.error("ports_map cannot not be empty, please check all links")
+            raise EnvironmentError("ports_map cannot be empty")
 
         # initialize virtualization resource pool
         self.virt_pool = VirtResource(self)
-- 
2.7.4

Re: [dts] [PATCH 1/4] framework/crb: Fixing ThunderX ethernet controler detection

[Radoslaw Biernacki]

[-- Attachment #1: Type: text/plain, Size: 2151 bytes --]

Ayuj and Jogarao you might also be interested to look at this one.

On 1 December 2017 at 22:20, Radoslaw Biernacki <
radoslaw.biernacki@linaro.org> wrote:

> Asking for link speed for ThunderX Ethernet controller is not reliable
> since driver report error when the link is down. In fact we dont need
> to ask for link speed as Ethernet controllers can be easily identified
> by device name from lspci. The mapping will fuhrer filter out the PF
> and VF interfaces which does not have the interface name assigned.
>
> Fixes: 150716d93f5e ("framework crb: Appending only 10G devices for
> cavium")
>
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
> ---
>  framework/crb.py | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/framework/crb.py b/framework/crb.py
> index dd29a8b..36b1ffe 100644
> --- a/framework/crb.py
> +++ b/framework/crb.py
> @@ -268,20 +268,13 @@ class Crb(object):
>          Look for the NIC's information (PCI Id and card type).
>          """
>          out = self.send_expect(
> -            "lspci -Dnn | grep -i eth", "# ", alt_session=True)
> +            "lspci -Dnn | grep -i 'Ethernet controller'", "# ",
> alt_session=True)
>          rexp = r"([\da-f]{4}:[\da-f]{2}:[\da-f]{2}.\d{1}) .*Eth.*?ernet
> .*?([\da-f]{4}:[\da-f]{4})"
>          pattern = re.compile(rexp)
>          match = pattern.findall(out)
>          self.pci_devices_info = []
>          for i in range(len(match)):
> -            #check if device is cavium and check its linkspeed, append
> only if it is 10G
> -            if "177d:" in match[i][1]:
> -                linkspeed = "10000"
> -                nic_linkspeed = self.send_command("cat
> /sys/bus/pci/devices/%s/net/*/speed" % match[i][0])
> -                if nic_linkspeed == linkspeed:
> -                    self.pci_devices_info.append((match[i][0],
> match[i][1]))
> -            else:
> -                self.pci_devices_info.append((match[i][0], match[i][1]))
> +            self.pci_devices_info.append((match[i][0], match[i][1]))
>
>      def pci_devices_information_uncached_freebsd(self):
>          """
> --
> 2.7.4
>
>

[-- Attachment #2: Type: text/html, Size: 2983 bytes --]

Re: [dts] [PATCH 1/4] framework/crb: Fixing ThunderX ethernet controler detection

[Radoslaw Biernacki]

[-- Attachment #1: Type: text/plain, Size: 3793 bytes --]

Hi,

I didn't noticed previously that this might be connected to my change so
thank for giving the feedback.

I need to spend some time and think about it, but is seems that "wirespeed"
function should be function of a nic or rather a port, not test class.
And I'm surprised that it didn't check the actual speed of a port since
even ThunderX NIC 10G can work either 1G or 10G.

So in general please give me some time I should came up with some more
elegant solution which possibly I can add as V2 of this patch.

On 4 December 2017 at 13:12, Verma, Ayuj <Ayuj.Verma@cavium.com> wrote:

> Hi Biernacki,
>
>
> Thanks for the heads up.
>
> The reason for us to put this check is ThunderX Ethernet controllers 1G,
> 10G and 40G NIC devices have same device-id which make it difficult to
> recognize correct device being tested, which is required in ./framework/
> test_case.py  "wirespeed" to provide bitrate for particular NIC.
>
>
> Initially we added support for 10G only but, we planed to have support for
> our 40G NIC also.
>
>
> More acceptable way to do this might be having a global variable for
> linkspeed and using it further in
> ./framework/test_case.py or elsewhere.
> User can provide linkspeed.
>
> Let us know your thoughts on this.
>
> Thanks and regards
> Ayuj Verma
>
>
>
>
>
> ------------------------------
> *From:* Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
> *Sent:* 04 December 2017 16:14
> *To:* dts@dpdk.org; Verma, Ayuj; Jogarao, Nartu
> *Cc:* Herbert Guan; Radoslaw Biernacki; Czubak, Angela; Liu, Yong
> *Subject:* Re: [PATCH 1/4] framework/crb: Fixing ThunderX ethernet
> controler detection
>
> Ayuj and Jogarao you might also be interested to look at this one.
>
> On 1 December 2017 at 22:20, Radoslaw Biernacki <
> radoslaw.biernacki@linaro.org> wrote:
>
> Asking for link speed for ThunderX Ethernet controller is not reliable
> since driver report error when the link is down. In fact we dont need
> to ask for link speed as Ethernet controllers can be easily identified
> by device name from lspci. The mapping will fuhrer filter out the PF
> and VF interfaces which does not have the interface name assigned.
>
> Fixes: 150716d93f5e ("framework crb: Appending only 10G devices for
> cavium")
>
> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>
> ---
>  framework/crb.py | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/framework/crb.py b/framework/crb.py
> index dd29a8b..36b1ffe 100644
> --- a/framework/crb.py
> +++ b/framework/crb.py
> @@ -268,20 +268,13 @@ class Crb(object):
>          Look for the NIC's information (PCI Id and card type).
>          """
>          out = self.send_expect(
> -            "lspci -Dnn | grep -i eth", "# ", alt_session=True)
> +            "lspci -Dnn | grep -i 'Ethernet controller'", "# ",
> alt_session=True)
>          rexp = r"([\da-f]{4}:[\da-f]{2}:[\da-f]{2}.\d{1}) .*Eth.*?ernet
> .*?([\da-f]{4}:[\da-f]{4})"
>          pattern = re.compile(rexp)
>          match = pattern.findall(out)
>          self.pci_devices_info = []
>          for i in range(len(match)):
> -            #check if device is cavium and check its linkspeed, append
> only if it is 10G
> -            if "177d:" in match[i][1]:
> -                linkspeed = "10000"
> -                nic_linkspeed = self.send_command("cat
> /sys/bus/pci/devices/%s/net/*/speed" % match[i][0])
> -                if nic_linkspeed == linkspeed:
> -                    self.pci_devices_info.append((match[i][0],
> match[i][1]))
> -            else:
> -                self.pci_devices_info.append((match[i][0], match[i][1]))
> +            self.pci_devices_info.append((match[i][0], match[i][1]))
>
>      def pci_devices_information_uncached_freebsd(self):
>          """
> --
> 2.7.4
>
>
>

[-- Attachment #2: Type: text/html, Size: 6585 bytes --]

Re: [dts] [PATCH 1/4] framework/crb: Fixing ThunderX ethernet controler detection

[Liu, Yong]

[-- Attachment #1: Type: text/plain, Size: 4777 bytes --]

Hi Biernacki,
“wirespeed” function is used by performance case for calculating the percentage of line rate. And by default we assume that only highest link speed will be used for performance case. So currently speed is hard-coded based on NIC type.
Agreed with you, port link speed should be one attribute of port which may be detected from tester side.

Thanks,
Marvin

From: Radoslaw Biernacki [mailto:radoslaw.biernacki@linaro.org]
Sent: Monday, December 04, 2017 9:58 PM
To: Verma, Ayuj <Ayuj.Verma@cavium.com>
Cc: dts@dpdk.org; Jogarao, Nartu <Nartu.Jogarao@cavium.com>; Herbert Guan <herbert.guan@linaro.org>; Czubak, Angela <Angela.Czubak@cavium.com>; Liu, Yong <yong.liu@intel.com>; Desai, Arvind <Arvind.Desai@cavium.com>; Athreya, Narayana Prasad <NarayanaPrasad.Athreya@cavium.com>
Subject: Re: [PATCH 1/4] framework/crb: Fixing ThunderX ethernet controler detection

Hi,

I didn't noticed previously that this might be connected to my change so thank for giving the feedback.

I need to spend some time and think about it, but is seems that "wirespeed" function should be function of a nic or rather a port, not test class.
And I'm surprised that it didn't check the actual speed of a port since even ThunderX NIC 10G can work either 1G or 10G.

So in general please give me some time I should came up with some more elegant solution which possibly I can add as V2 of this patch.

On 4 December 2017 at 13:12, Verma, Ayuj <Ayuj.Verma@cavium.com<mailto:Ayuj.Verma@cavium.com>> wrote:

Hi Biernacki,



Thanks for the heads up.

The reason for us to put this check is ThunderX Ethernet controllers 1G, 10G and 40G NIC devices have same device-id which make it difficult to recognize correct device being tested, which is required in ./framework/test_case.py  "wirespeed" to provide bitrate for particular NIC.



Initially we added support for 10G only but, we planed to have support for our 40G NIC also.



More acceptable way to do this might be having a global variable for linkspeed and using it further in
./framework/test_case.py or elsewhere.
User can provide linkspeed.

Let us know your thoughts on this.

Thanks and regards
Ayuj Verma






________________________________
From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org<mailto:radoslaw.biernacki@linaro.org>>
Sent: 04 December 2017 16:14
To: dts@dpdk.org<mailto:dts@dpdk.org>; Verma, Ayuj; Jogarao, Nartu
Cc: Herbert Guan; Radoslaw Biernacki; Czubak, Angela; Liu, Yong
Subject: Re: [PATCH 1/4] framework/crb: Fixing ThunderX ethernet controler detection

Ayuj and Jogarao you might also be interested to look at this one.

On 1 December 2017 at 22:20, Radoslaw Biernacki <radoslaw.biernacki@linaro.org<mailto:radoslaw.biernacki@linaro.org>> wrote:
Asking for link speed for ThunderX Ethernet controller is not reliable
since driver report error when the link is down. In fact we dont need
to ask for link speed as Ethernet controllers can be easily identified
by device name from lspci. The mapping will fuhrer filter out the PF
and VF interfaces which does not have the interface name assigned.

Fixes: 150716d93f5e ("framework crb: Appending only 10G devices for cavium")

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org<mailto:radoslaw.biernacki@linaro.org>>
---
 framework/crb.py | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/framework/crb.py b/framework/crb.py
index dd29a8b..36b1ffe 100644
--- a/framework/crb.py
+++ b/framework/crb.py
@@ -268,20 +268,13 @@ class Crb(object):
         Look for the NIC's information (PCI Id and card type).
         """
         out = self.send_expect(
-            "lspci -Dnn | grep -i eth", "# ", alt_session=True)
+            "lspci -Dnn | grep -i 'Ethernet controller'", "# ", alt_session=True)
         rexp = r"([\da-f]{4}:[\da-f]{2}:[\da-f]{2}.\d{1}) .*Eth.*?ernet .*?([\da-f]{4}:[\da-f]{4})"
         pattern = re.compile(rexp)
         match = pattern.findall(out)
         self.pci_devices_info = []
         for i in range(len(match)):
-            #check if device is cavium and check its linkspeed, append only if it is 10G
-            if "177d:" in match[i][1]:
-                linkspeed = "10000"
-                nic_linkspeed = self.send_command("cat /sys/bus/pci/devices/%s/net/*/speed" % match[i][0])
-                if nic_linkspeed == linkspeed:
-                    self.pci_devices_info.append((match[i][0], match[i][1]))
-            else:
-                self.pci_devices_info.append((match[i][0], match[i][1]))
+            self.pci_devices_info.append((match[i][0], match[i][1]))

     def pci_devices_information_uncached_freebsd(self):
         """
--
2.7.4



[-- Attachment #2: Type: text/html, Size: 14262 bytes --]