[dts] [PATCH V1] fix link status interrupt failed case

[Xu, HuilongX]

Hi  yong,

-----Original Message-----
From: Liu, Yong 
Sent: Wednesday, August 30, 2017 5:07 PM
To: Xu, HuilongX <huilongx.xu at intel.com>; dts at dpdk.org
Cc: Xu, HuilongX <huilongx.xu at intel.com>
Subject: RE: [dts] [PATCH V1] fix link status interrupt failed case

Huilong,
Some comments and questions below.

> -----Original Message-----
> From: dts [mailto:dts-bounces at dpdk.org] On Behalf Of xu,huilong
> Sent: Friday, August 25, 2017 3:12 PM
> To: dts at dpdk.org
> Cc: Xu, HuilongX <huilongx.xu at intel.com>
> Subject: [dts] [PATCH V1] fix link status interrupt failed case
> 
> update list:
> 1. update fortville nic test interrupt mode list 2. fix example setup 
> failed 3. fix check port stats in example 4. fix check rx/tx packet 
> get count number error
> 
> Signed-off-by: xu,huilong <huilongx.xu at intel.com>
> ---
>  tests/TestSuite_link_status_interrupt.py | 34 
> +++++++++++++++++++++++----
> -----
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/TestSuite_link_status_interrupt.py
> b/tests/TestSuite_link_status_interrupt.py
> index ae205cc..2249cf1 100644
> --- a/tests/TestSuite_link_status_interrupt.py
> +++ b/tests/TestSuite_link_status_interrupt.py
> @@ -61,6 +61,15 @@ class TestLinkStatusInterrupt(TestCase):
>          out = self.dut.build_dpdk_apps("./examples/link_status_interrupt")
>          self.verify("Error" not in out, "compilation error 1")
>          self.verify("No such file" not in out, "compilation error 2")
> +        # fortville NIC not support legacy intr mode
> +        if self.nic in ["fortville_eagle", "fortville_spirit",
> +                        "fortville_spirt_single", "fortville_25g"]:
> +            self.basic_intr_mode = ["msix"]
> +        else:
> +            self.basic_intr_mode = ["msix", "legacy"]
> +        # vfio support msi intr mode
> +        if self.drivername == "vfio-pci":
> +            self.basic_intr_mode.append("msi")


Please add link of dpdk release note in the commit log.
This is a linux kernel issue, I will add kernel patch url next version

> 
>      def set_link_status_and_verify(self, dutPort, status):
>          """
> @@ -71,7 +80,9 @@ class TestLinkStatusInterrupt(TestCase):
>          self.tester.send_expect("ifconfig %s %s" %
>                                  (self.intf, status.lower()), "# ", 10)
>          verify_point = "Port %s Link %s" % (dutPort, status)
> -        self.dut.send_expect("", verify_point, 60)
> +
> +        out = self.dut.get_session_output(timeout=60)
> +        self.verify(verify_point in out,"link status update error")
> 
>      def set_up(self):
>          """
> @@ -86,20 +97,23 @@ class TestLinkStatusInterrupt(TestCase):
>          if self.drivername == "igb_uio":
>              cmdline = self.path + " -c %s -n %s -- -p %s " % (
>                  self.coremask, self.dut.get_memory_channels(),
> self.portmask)
> -            for mode in ["legacy", "msix"]:
> +            for mode in self.basic_intr_mode:
>                  self.dut.send_expect("rmmod -f igb_uio", "#", 20)
>                  self.dut.send_expect(
>                      'insmod %s/kmod/igb_uio.ko "intr_mode=%s"' % 
> (self.target, mode), "# ")
>                  self.dut.bind_interfaces_linux()
> -                self.dut.send_expect(cmdline, "Aggregate statistics", 60)
> +                self.dut.send_expect(cmdline, "", 180)

If there's nothing to check in output, you can use function "self.dut.send_command(timeout=time)".
And 180 seconds is too long, what is waiting for?
I will use send_command to replace send_expect, and example setup need more time, because this example will print info in loop mode.

> +                out = self.dut.get_session_output(timeout=60)

Function "get_session_output" just return all output after timeout. Normally we will use default value, why enlarge time out value to 60 seconds?
Infor "Port statistics" form example by loop mode, so need more time.
> +                self.verify("Port statistics" in out, "setup example
> error")
> +                time.sleep(10)
>                  self.set_link_status_and_verify(self.dut_ports[0], 'Down')
>                  self.set_link_status_and_verify(self.dut_ports[0], 'Up')
>                  self.dut.send_expect("^C", "#", 60)
>          elif self.drivername == "vfio-pci":
> -            for mode in ["legacy", "msi", "msix"]:
> +            for mode in self.basic_intr_mode :

Pep issue here, please modify it.
I will update it at next version
>                  cmdline = self.path + " -c %s -n %s --vfio-intr=%s -- 
> - p %s" % (
>                      self.coremask, self.dut.get_memory_channels(), 
> mode,
> self.portmask)
> -                self.dut.send_expect(cmdline, "Aggregate statistics", 60)
> +                self.dut.send_expect(cmdline, "statistics 
> + statistics",
> 120)


A bit tricky here, why double "statistics" here?
I will update it next version
>                  self.set_link_status_and_verify(self.dut_ports[0], 'Down')
>                  self.set_link_status_and_verify(self.dut_ports[0], 'Up')
>                  self.dut.send_expect("^C", "#", 60) @@ -112,7 +126,7 
> @@ class TestLinkStatusInterrupt(TestCase):
>          if self.drivername == "igb_uio":
>              cmdline = self.path + " -c %s -n %s -- -p %s " % (
>                  self.coremask, self.dut.get_memory_channels(),
> self.portmask)
> -            for mode in ["legacy", "msix"]:
> +            for mode in self.basic_intr_mode:
>                  self.dut.send_expect("rmmod -f igb_uio", "#", 20)
>                  self.dut.send_expect(
>                      'insmod %s/kmod/igb_uio.ko "intr_mode=%s"' % 
> (self.target, mode), "# ") @@ -129,11 +143,12 @@ class 
> TestLinkStatusInterrupt(TestCase):
>                  pkt_send = re.findall("Total packets sent:\s*(\d*)", out)
>                  pkt_received = re.findall(
>                      "Total packets received:\s*(\d*)", out)
> -                self.verify(pkt_send == pkt_received == '1',
> +                # example will print rx/tx packet info by loop so 
> + only
> check last one
> +                self.verify(pkt_send[-1] == pkt_received[-1] == '1',
>                              "Error: sent packets != received packets")
>                  self.dut.send_expect("^C", "#", 60)
>          elif self.drivername == "vfio-pci":
> -            for mode in ["legacy", "msi", "msix"]:
> +            for mode in self.basic_intr_mode:
>                  cmdline = self.path + " -c %s -n %s --vfio-intr=%s -- 
> - p %s" % (
>                      self.coremask, self.dut.get_memory_channels(), 
> mode,
> self.portmask)
>                  self.dut.send_expect(cmdline, "Aggregate statistics", 
> 60) @@ -148,7 +163,8 @@ class TestLinkStatusInterrupt(TestCase):
>                  pkt_send = re.findall("Total packets sent:\s*(\d*)", out)
>                  pkt_received = re.findall(
>                      "Total packets received:\s*(\d*)", out)
> -                self.verify(pkt_send == pkt_received == '1',
> +                # example will print rx/tx packet info by loop so 
> + only
> check last one
> +                self.verify(pkt_send[-1] == pkt_received[-1] == '1',
>                              "Error: sent packets != received packets")
>                  self.dut.send_expect("^C", "#", 60)
> 
> --
> 1.9.3