rx interrupt: Fixed test case
diff mbox series

Message ID 20200826150429.278284-1-ohilyard@iol.unh.edu
State New
Headers show
Series
  • rx interrupt: Fixed test case
Related show

Commit Message

Owen Hilyard Aug. 26, 2020, 3:04 p.m. UTC
fixed test case issues with eal params
removed extra instances of l3fwd-power

Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
---
 test_plans/interrupt_pmd_test_plan.rst | 58 +++++++++-----------
 tests/TestSuite_interrupt_pmd.py       | 73 ++++++++++++++------------
 2 files changed, 64 insertions(+), 67 deletions(-)

Comments

Ma, LihongX Sept. 2, 2020, 2:01 a.m. UTC | #1
Hi, Owen
I think the change of the plan is not make sense, the case ' PF interrupt pmd with different queue' is want to test the interrupt on different queue,
The original case will test the queue on min number, max number and normal number(between minimum and maximum), but your patch will only test one situation.


Regards,
Ma,lihong

> -----Original Message-----
> From: dts <dts-bounces@dpdk.org> On Behalf Of Owen Hilyard
> Sent: Wednesday, August 26, 2020 11:04 PM
> To: dts@dpdk.org
> Cc: Zhang, Yuwei1 <yuwei1.zhang@intel.com>; changqingx.wu@intel.com; Xiao,
> QimaiX <qimaix.xiao@intel.com>; Hunt, David <david.hunt@intel.com>;
> lylavoie@iol.unh.edu; Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [dts] [PATCH] rx interrupt: Fixed test case
> 
> fixed test case issues with eal params
> removed extra instances of l3fwd-power
> 
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---
>  test_plans/interrupt_pmd_test_plan.rst | 58 +++++++++-----------
>  tests/TestSuite_interrupt_pmd.py       | 73 ++++++++++++++------------
>  2 files changed, 64 insertions(+), 67 deletions(-)
> 
> diff --git a/test_plans/interrupt_pmd_test_plan.rst
> b/test_plans/interrupt_pmd_test_plan.rst
> index cb8b2f1..1f8816d 100644
> --- a/test_plans/interrupt_pmd_test_plan.rst
> +++ b/test_plans/interrupt_pmd_test_plan.rst
Owen Hilyard Sept. 2, 2020, 6:42 p.m. UTC | #2
Hello

I'm able to see a material difference between what I've suggested and what
the prior test case did. I was attempting to reduce the number of
invocations of a pmd during the test, since those invocations are time
consuming and, from what I measured, made up the majority of the runtime of
the test. Is there a reason why all queues and port's can't be opened at
the same time and then ignored until they are needed? The way I re-did the
configs was designed to create all possible combinations of settings in the
format that was originally there. Are all 3 invocations of the pmd needed
or is it possible to merge those and throw out my other changes? Most of my
changes were done because I was already planning on submitting a patch to
remove the extra invocations and aren't as important.

Thanks for your help
Owen

On Tue, Sep 1, 2020 at 10:02 PM Ma, LihongX <lihongx.ma@intel.com> wrote:

> Hi, Owen
> I think the change of the plan is not make sense, the case ' PF interrupt
> pmd with different queue' is want to test the interrupt on different queue,
> The original case will test the queue on min number, max number and normal
> number(between minimum and maximum), but your patch will only test one
> situation.
>
>
> Regards,
> Ma,lihong
>
> > -----Original Message-----
> > From: dts <dts-bounces@dpdk.org> On Behalf Of Owen Hilyard
> > Sent: Wednesday, August 26, 2020 11:04 PM
> > To: dts@dpdk.org
> > Cc: Zhang, Yuwei1 <yuwei1.zhang@intel.com>; changqingx.wu@intel.com;
> Xiao,
> > QimaiX <qimaix.xiao@intel.com>; Hunt, David <david.hunt@intel.com>;
> > lylavoie@iol.unh.edu; Owen Hilyard <ohilyard@iol.unh.edu>
> > Subject: [dts] [PATCH] rx interrupt: Fixed test case
> >
> > fixed test case issues with eal params
> > removed extra instances of l3fwd-power
> >
> > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > ---
> >  test_plans/interrupt_pmd_test_plan.rst | 58 +++++++++-----------
> >  tests/TestSuite_interrupt_pmd.py       | 73 ++++++++++++++------------
> >  2 files changed, 64 insertions(+), 67 deletions(-)
> >
> > diff --git a/test_plans/interrupt_pmd_test_plan.rst
> > b/test_plans/interrupt_pmd_test_plan.rst
> > index cb8b2f1..1f8816d 100644
> > --- a/test_plans/interrupt_pmd_test_plan.rst
> > +++ b/test_plans/interrupt_pmd_test_plan.rst
>
>
Lijuan Tu Sept. 9, 2020, 5:58 a.m. UTC | #3
Hi Owen,

Reduce the number of invocations is a good idea, and your design is more perfect for a common case. But we still need to consider the boundary, the minimum and the maximum queue number. I really suggest we might get a random number from the minimum, maximum, and normal queue number, if then invocation is reduced, besides boundary checking is covered. Definitely we will run test for a long time not only once.

thanks

From: dts <dts-bounces@dpdk.org> On Behalf Of Owen Hilyard
Sent: 2020年9月3日 2:42
To: Ma, LihongX <lihongx.ma@intel.com>
Cc: dts@dpdk.org; Zhang, Yuwei1 <yuwei1.zhang@intel.com>; changqingx.wu@intel.com; Xiao, QimaiX <qimaix.xiao@intel.com>; Hunt, David <david.hunt@intel.com>; lylavoie@iol.unh.edu
Subject: Re: [dts] [PATCH] rx interrupt: Fixed test case

Hello
I'm able to see a material difference between what I've suggested and what the prior test case did. I was attempting to reduce the number of invocations of a pmd during the test, since those invocations are time consuming and, from what I measured, made up the majority of the runtime of the test. Is there a reason why all queues and port's can't be opened at the same time and then ignored until they are needed? The way I re-did the configs was designed to create all possible combinations of settings in the format that was originally there. Are all 3 invocations of the pmd needed or is it possible to merge those and throw out my other changes? Most of my changes were done because I was already planning on submitting a patch to remove the extra invocations and aren't as important.

Thanks for your help
Owen

On Tue, Sep 1, 2020 at 10:02 PM Ma, LihongX <lihongx.ma@intel.com<mailto:lihongx.ma@intel.com>> wrote:
Hi, Owen
I think the change of the plan is not make sense, the case ' PF interrupt pmd with different queue' is want to test the interrupt on different queue,
The original case will test the queue on min number, max number and normal number(between minimum and maximum), but your patch will only test one situation.


Regards,
Ma,lihong

> -----Original Message-----
> From: dts <dts-bounces@dpdk.org<mailto:dts-bounces@dpdk.org>> On Behalf Of Owen Hilyard
> Sent: Wednesday, August 26, 2020 11:04 PM
> To: dts@dpdk.org<mailto:dts@dpdk.org>
> Cc: Zhang, Yuwei1 <yuwei1.zhang@intel.com<mailto:yuwei1.zhang@intel.com>>; changqingx.wu@intel.com<mailto:changqingx.wu@intel.com>; Xiao,
> QimaiX <qimaix.xiao@intel.com<mailto:qimaix.xiao@intel.com>>; Hunt, David <david.hunt@intel.com<mailto:david.hunt@intel.com>>;
> lylavoie@iol.unh.edu<mailto:lylavoie@iol.unh.edu>; Owen Hilyard <ohilyard@iol.unh.edu<mailto:ohilyard@iol.unh.edu>>
> Subject: [dts] [PATCH] rx interrupt: Fixed test case
>
> fixed test case issues with eal params
> removed extra instances of l3fwd-power
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu<mailto:ohilyard@iol.unh.edu>>
> ---
>  test_plans/interrupt_pmd_test_plan.rst | 58 +++++++++-----------
>  tests/TestSuite_interrupt_pmd.py       | 73 ++++++++++++++------------
>  2 files changed, 64 insertions(+), 67 deletions(-)
>
> diff --git a/test_plans/interrupt_pmd_test_plan.rst
> b/test_plans/interrupt_pmd_test_plan.rst
> index cb8b2f1..1f8816d 100644
> --- a/test_plans/interrupt_pmd_test_plan.rst
> +++ b/test_plans/interrupt_pmd_test_plan.rst
Owen Hilyard Sept. 11, 2020, 8:17 p.m. UTC | #4
After looking into it, the l3fwd-power tool does not have the capability to
change queue numbers or mappings without restarting. Testpmd currently
lacks the ability (as far as I know) to bind a queue to an lcore. Is this a
feature that would be reasonable to add to testpmd or should I drop work on
this test case? This is turning into quite a time sink and I'm beginning to
think that it would be more beneficial to DTS if I focused on creating the
Flow API test suite rather than trying to make marginal improvements to
this test case.

Owen Hilyard

On Wed, Sep 9, 2020 at 1:58 AM Tu, Lijuan <lijuan.tu@intel.com> wrote:

> Hi Owen,
>
>
>
> Reduce the number of invocations is a good idea, and your design is more
> perfect for a common case. But we still need to consider the boundary, the
> minimum and the maximum queue number. I really suggest we might get a
> random number from the minimum, maximum, and normal queue number, if then
> invocation is reduced, besides boundary checking is covered. Definitely we
> will run test for a long time not only once.
>
>
>
> thanks
>
>
>
> *From:* dts <dts-bounces@dpdk.org> *On Behalf Of * Owen Hilyard
> *Sent:* 2020年9月3日 2:42
> *To:* Ma, LihongX <lihongx.ma@intel.com>
> *Cc:* dts@dpdk.org; Zhang, Yuwei1 <yuwei1.zhang@intel.com>;
> changqingx.wu@intel.com; Xiao, QimaiX <qimaix.xiao@intel.com>; Hunt,
> David <david.hunt@intel.com>; lylavoie@iol.unh.edu
> *Subject:* Re: [dts] [PATCH] rx interrupt: Fixed test case
>
>
>
> Hello
>
> I'm able to see a material difference between what I've suggested and what
> the prior test case did. I was attempting to reduce the number of
> invocations of a pmd during the test, since those invocations are time
> consuming and, from what I measured, made up the majority of the runtime of
> the test. Is there a reason why all queues and port's can't be opened at
> the same time and then ignored until they are needed? The way I re-did the
> configs was designed to create all possible combinations of settings in the
> format that was originally there. Are all 3 invocations of the pmd needed
> or is it possible to merge those and throw out my other changes? Most of my
> changes were done because I was already planning on submitting a patch to
> remove the extra invocations and aren't as important.
>
> Thanks for your help
>
> Owen
>
>
>
> On Tue, Sep 1, 2020 at 10:02 PM Ma, LihongX <lihongx.ma@intel.com> wrote:
>
> Hi, Owen
> I think the change of the plan is not make sense, the case ' PF interrupt
> pmd with different queue' is want to test the interrupt on different queue,
> The original case will test the queue on min number, max number and normal
> number(between minimum and maximum), but your patch will only test one
> situation.
>
>
> Regards,
> Ma,lihong
>
> > -----Original Message-----
> > From: dts <dts-bounces@dpdk.org> On Behalf Of Owen Hilyard
> > Sent: Wednesday, August 26, 2020 11:04 PM
> > To: dts@dpdk.org
> > Cc: Zhang, Yuwei1 <yuwei1.zhang@intel.com>; changqingx.wu@intel.com;
> Xiao,
> > QimaiX <qimaix.xiao@intel.com>; Hunt, David <david.hunt@intel.com>;
> > lylavoie@iol.unh.edu; Owen Hilyard <ohilyard@iol.unh.edu>
> > Subject: [dts] [PATCH] rx interrupt: Fixed test case
> >
> > fixed test case issues with eal params
> > removed extra instances of l3fwd-power
> >
> > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > ---
> >  test_plans/interrupt_pmd_test_plan.rst | 58 +++++++++-----------
> >  tests/TestSuite_interrupt_pmd.py       | 73 ++++++++++++++------------
> >  2 files changed, 64 insertions(+), 67 deletions(-)
> >
> > diff --git a/test_plans/interrupt_pmd_test_plan.rst
> > b/test_plans/interrupt_pmd_test_plan.rst
> > index cb8b2f1..1f8816d 100644
> > --- a/test_plans/interrupt_pmd_test_plan.rst
> > +++ b/test_plans/interrupt_pmd_test_plan.rst
>
>

Patch
diff mbox series

diff --git a/test_plans/interrupt_pmd_test_plan.rst b/test_plans/interrupt_pmd_test_plan.rst
index cb8b2f1..1f8816d 100644
--- a/test_plans/interrupt_pmd_test_plan.rst
+++ b/test_plans/interrupt_pmd_test_plan.rst
@@ -1,4 +1,5 @@ 
 .. Copyright (c) <2017>, Intel Corporation
+   Copyright © 2018[, 2019] The University of New Hampshire. All rights reserved.
    All rights reserved.
 
    Redistribution and use in source and binary forms, with or without
@@ -63,47 +64,36 @@  in bios. When used vfio, requested to insmod two drivers vfio and vfio-pci.
 Test Case1: PF interrupt pmd with different queue
 =================================================
 
-Run l3fwd-power with one queue per port::
-
-    l3fwd-power -c 0x7 -n 4 -- -p 0x3 -P --config="(0,0,1),(1,0,2)"
-
-Send one packet to Port0 and Port1, check that thread on core1 and core2
-waked up::
-
-    L3FWD_POWER: lcore 1 is waked up from rx interrupt on port1,rxq0
-    L3FWD_POWER: lcore 2 is waked up from rx interrupt on port1,rxq0
-
-Check the packet has been normally forwarded.
-
-After the packet forwarded, thread on core1 and core 2 will return to sleep::
+Run l3fwd-power with each port having an lcore assigned to it and 4 queues per core ::
 
-    L3FWD_POWER: lcore 1 sleeps until interrupt on port0,rxq0 triggers
-    L3FWD_POWER: lcore 2 sleeps until interrupt on port0,rxq0 triggers
+    l3fwd-power -l 1,2,3,4 -n 4 -w 0000:88:00.0 -w 0000:88:00.1 -- -p 0x3 -P \
+      --config='(0, 0, 1),(0, 1, 1),(0, 2, 1),(0, 3, 1),(1, 0, 2),(1, 1, 2),(1, 2, 2),(1, 3, 2)'
 
-Send packet flows to Port0 and Port1, check that thread on core1 and core2 will
-keep up awake.
+Send one packet per option in the config flag ::
 
-Run l3fwd-power with random number queue per port, if is 4::
+   L3FWD_POWER: lcore 1 is waked up from rx interrupt on port 0 queue 1
+   L3FWD_POWER: lcore 1 sleeps until interrupt triggers
+   L3FWD_POWER: lcore 1 is waked up from rx interrupt on port 0 queue 0
+   L3FWD_POWER: lcore 1 sleeps until interrupt triggers
+   L3FWD_POWER: lcore 1 is waked up from rx interrupt on port 0 queue 3
+   L3FWD_POWER: lcore 1 sleeps until interrupt triggers
+   L3FWD_POWER: lcore 1 is waked up from rx interrupt on port 0 queue 2
+   L3FWD_POWER: lcore 1 sleeps until interrupt triggers
+   L3FWD_POWER: lcore 2 is waked up from rx interrupt on port 1 queue 1
+   L3FWD_POWER: lcore 2 sleeps until interrupt triggers
+   L3FWD_POWER: lcore 2 is waked up from rx interrupt on port 1 queue 0
+   L3FWD_POWER: lcore 2 sleeps until interrupt triggers
+   L3FWD_POWER: lcore 2 is waked up from rx interrupt on port 1 queue 3
+   L3FWD_POWER: lcore 2 sleeps until interrupt triggers
+   L3FWD_POWER: lcore 2 is waked up from rx interrupt on port 1 queue 2
+   L3FWD_POWER: lcore 2 sleeps until interrupt triggers
 
-    l3fwd-power -c 0x7 -n 4 -- -p 0x3 -P --config="0,0,0),(0,1,1),\
-       (0,2,2),(0,3,3),(0,4,4)"
+Check that all configured combinations of port, queue and lcore have been used.
 
-Send packet with increased dest IP to Port0, check that all threads waked up
-
-Send packet flows to Port0 and Port1, check that thread on core1 and core2 will
-keep up awake.
-
-Run l3fwd-power with 15 queues per port::
-
-    l3fwd-power -c 0xffffff -n 4 -- -p 0x3 -P --config="(0,0,0),(0,1,1),\
-        (0,2,2),(0,3,3),(0,4,4),(0,5,5),(0,6,6),(0,7,7),(1,0,8),\
-        (1,1,9),(1,2,10),(1,3,11),(1,4,12),(1,5,13),(1,6,14)"
-
-Send packet with increased dest IP to Port0, check that all threads waked up
+Check that all cores went back to sleep.
 
 igb_uio driver only uses one queue 0
 
-
 Test Case2: PF lsc interrupt with vfio
 ======================================
 
@@ -135,4 +125,4 @@  Compare latency(l3fwd-power PF interrupt pmd with uio) with l3fwd latency.
 
 Setup validation scenario the case as test2
 Send burst packet flow to Port0 and Port1, use IXIA capture the maximum
-latency.
+latency.
\ No newline at end of file
diff --git a/tests/TestSuite_interrupt_pmd.py b/tests/TestSuite_interrupt_pmd.py
index 17a58af..3fb43e4 100644
--- a/tests/TestSuite_interrupt_pmd.py
+++ b/tests/TestSuite_interrupt_pmd.py
@@ -1,6 +1,7 @@ 
 # BSD LICENSE
 #
-# Copyright(c) 2010-2019 Intel Corporation. All rights reserved.
+# Copyright(c) 2020 Intel Corporation. All rights reserved.
+# Copyright © 2018[, 2019] The University of New Hampshire. All rights reserved.
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
@@ -46,20 +47,15 @@  class TestInterruptPmd(TestCase):
         """
         Run at the start of each test suite.
         """
+        self.dut.send_expect("modprobe msr", "# ")
+        self.num_queues = 4
 
         self.dut_ports = self.dut.get_ports(self.nic)
         self.verify(len(self.dut_ports) >= 2, "Insufficient ports")
-        cores = self.dut.get_core_list("1S/4C/1T")
-        self.coremask = utils.create_mask(cores)
+        self.cores = self.dut.get_core_list("1S/4C/1T")
+        self.coremask = utils.create_mask(self.cores)
 
         self.path = "./examples/l3fwd-power/build/l3fwd-power"
-
-        self.trafficFlow = {
-            "Flow1": [[0, 0, 1], [1, 0, 2]],
-            "Flow2": [[0, 0, 0], [0, 1, 1], [0, 2, 2], [0, 3, 3], [0, 4, 4]],
-            "Flow3": [[0, 0, 0], [0, 1, 1], [0, 2, 2], [0, 3, 3], [0, 4, 4], [0, 5, 5], [0, 6, 6], [0, 7, 7],
-                      [1, 0, 8], [1, 1, 9], [1, 2, 10], [1, 3, 11], [1, 4, 12], [1, 5, 13], [1, 6, 14]],
-        }
         # build sample app
         out = self.dut.build_dpdk_apps("./examples/l3fwd-power")
         self.verify("Error" not in out, "compilation error 1")
@@ -88,35 +84,45 @@  class TestInterruptPmd(TestCase):
         pass
 
     def test_different_queue(self):
-        cmd = "%s -c %s -n 4 -- -p 0x3 -P --config='(0,0,1),(1,0,2)' "% (self.path, self.coremask)
-        self.dut.send_expect(cmd, "L3FWD_POWER", 60)
-        portQueueLcore = self.trafficFlow["Flow1"]
-        self.verifier_result(2, 2, portQueueLcore)
+        lcore_configs = [
+            (port, queue, lcore + 1) for lcore, port in enumerate(self.dut_ports) for queue in
+            range(self.num_queues)
+        ]
 
-        self.dut.kill_all()
-        cmd = "%s -c 0x3f -n 4 -- -p 0x3 -P --config='(0,0,0),(0,1,1),(0,2,2),(0,3,3),(0,4,4)' "% self.path
-        self.dut.send_expect(cmd, "L3FWD_POWER", 120)
-        portQueueLcore = self.trafficFlow["Flow2"]
-        self.verifier_result(20, 1, portQueueLcore)
+        port_mask = utils.create_mask(self.dut_ports)
+        config = ','.join(str(cfg_group) for cfg_group in lcore_configs)
 
-        self.dut.kill_all()
-        cmd = "%s -c 0xffffff -n 4 -- -p 0x3 -P --config='(0,0,0),(0,1,1),(0,2,2),(0,3,3),\
-        (0,4,4),(0,5,5),(0,6,6),(0,7,7),(1,0,8),(1,1,9),(1,2,10),(1,3,11),\
-        (1,4,12),(1,5,13),(1,6,14)' "% self.path
+        eal_config = {
+            'cores': self.cores,
+            'ports': [self.dut.ports_info[i]['pci'] for i in range(len(self.dut.ports_info))]
+        }
+
+        all_eal_param = self.dut.create_eal_parameters(**eal_config)
+
+        cmd = f"{self.path} {all_eal_param} -- -p {port_mask} -P --config='{config}'"
 
         self.dut.send_expect(cmd, "L3FWD_POWER", 60)
-        portQueueLcore = self.trafficFlow["Flow3"]
-        self.verifier_result(40, 2, portQueueLcore)
+
+        self.verifier_result(self.num_queues, len(self.dut_ports), lcore_configs)
+
+        self.dut.kill_all()
 
     def verifier_result(self, num, portnum, portQueueLcore):
         self.scapy_send_packet(num, portnum)
-        result = self.dut.get_session_output(timeout=5)
-        for i in range(len(portQueueLcore)):
-            lcorePort = portQueueLcore[i]
-            self.verify("FWD_POWER: lcore %d is waked up from rx interrupt on port %d queue %d" %(lcorePort[2],
-                lcorePort[0], lcorePort[1]) in result, "Wrong: lcore %d is waked up failed" % lcorePort[2])
-            self.verify("L3FWD_POWER: lcore %d sleeps until interrupt triggers" %(
-                lcorePort[2]) in result, "Wrong: lcore %d not sleeps until interrupt triggers" % lcorePort[2])
+        result: str = self.dut.get_session_output(timeout=5)
+        for lcorePort in portQueueLcore:
+            wake_index = result.rfind(
+                f"FWD_POWER: lcore {lcorePort[2]:d} is waked up from rx interrupt on port "
+                f"{lcorePort[0]:d} queue {lcorePort[1]:d}")
+
+            self.verify(wake_index >= 0, f"Wrong: lcore {lcorePort[2]:d} is waked up failed")
+
+            sleep_index = result.rfind(f"L3FWD_POWER: lcore {lcorePort[2]:d} sleeps until interrupt triggers")
+            self.verify(sleep_index >= 0, f"Wrong: lcore {lcorePort[2]:d} not sleeps until interrupt triggers")
+
+            # Make sure that the sleep statement is after the last wake statement
+            self.verify(sleep_index > wake_index,
+                        f"Wrong: lcore {lcorePort[2]:d} not sleeps until interrupt triggers")
 
     def scapy_send_packet(self, num, portnum):
         """
@@ -128,7 +134,7 @@  class TestInterruptPmd(TestCase):
                 mac = self.dut.get_mac_address(self.dut_ports[i])
                 txItf = self.tester.get_interface(txport)
                 self.tester.scapy_append(
-                    'sendp([Ether()/IP(dst="198.0.0.%d")/UDP()/Raw(\'X\'*18)], iface="%s")' % (j, txItf))
+                    f'sendp([Ether(dst="{mac}")/IP(dst="198.0.0.{j:d}")/UDP()/Raw(\'X\'*18)], iface="{txItf}")')
         self.tester.scapy_execute()
 
     def tear_down(self):
@@ -141,5 +147,6 @@  class TestInterruptPmd(TestCase):
         """
         Run after each test suite.
         """
+        self.dut.send_expect("rmmod msr", "# ")
         self.dut.kill_all()
         self.set_nic_driver(self.default_driver)