Re: [dts] [PATCH 2/2] add link bond slave dynamic config test case

["Xu, HuilongX" <huilongx.xu@intel.com>]

Hi yong,

Thanks  a lot 

> -----Original Message-----
> From: Liu, Yong
> Sent: Friday, May 12, 2017 4:54 PM
> To: Xu, HuilongX; dts@dpdk.org
> Subject: Re: [dts] [PATCH 2/2] add link bond slave dynamic config test case
> 
> Huilong,
> Some comments below.
> 
> On 05/12/2017 03:44 PM, xu,huilong wrote:
> > update list:
> > 1 add link bond slave dynamic config test case
> > 2 use two port for this suite
> >
> > Signed-off-by: xu,huilong <huilongx.xu@intel.com>
> > ---
> >   tests/TestSuite_pmdrss_hash.py | 32
> +++++++++++++++++++++++++++++---
> >   1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/TestSuite_pmdrss_hash.py
> > b/tests/TestSuite_pmdrss_hash.py index 78ae34d..fd032f4 100644
> > --- a/tests/TestSuite_pmdrss_hash.py
> > +++ b/tests/TestSuite_pmdrss_hash.py
> > @@ -417,10 +417,10 @@ class TestPmdrssHash(TestCase):
> >           """
> >
> >           self.verify(self.nic in ["fortville_eagle", "fortville_spirit",
> > -                    "fortville_spirit_single", "redrockcanyou", "atwood",
> "boulderrapid", "fortpark_TLV"],
> > +                    "fortville_spirit_single", "redrockcanyou",
> > + "atwood", "boulderrapid", "fortpark_TLV","fortville_25g"],
> 
> Here contain pep8 issue, please check with pep8 tool.
Yes, I will update this in next version.
> >                       "NIC Unsupported: " + str(self.nic))
> >           global reta_num
> > -        if self.nic in ["fortville_eagle", "fortville_spirit", "fortville_spirit_single",
> "fortpark_TLV"]:
> > +        if self.nic in ["fortville_eagle", "fortville_spirit", "fortville_spirit_single",
> "fortpark_TLV","fortville_25g"]:
> >               reta_num = 512
> >           elif self.nic in ["niantic"]:
> >               reta_num = 128
> > @@ -429,7 +429,7 @@ class TestPmdrssHash(TestCase):
> >           else:
> >               self.verify(False, "NIC Unsupported:%s" % str(self.nic))
> >           ports = self.dut.get_ports(self.nic)
> > -        self.verify(len(ports) >= 1, "Not enough ports available")
> > +        self.verify(len(ports) >= 2, "Not enough ports available")
> We are trying to limit port usage, is this case must use two ports?
Yes, because this case must use two ports. One ports config rss by manual. Another port will auto sync rss config.

> 
> >
> >       def set_up(self):
> >           """
> > @@ -651,6 +651,32 @@ class TestPmdrssHash(TestCase):
> >
> >               self.dut.send_expect("quit", "# ", 30)
> >
> > +    def test_dynamic_rss_bond_config(self):
> > +        self.dut.send_expect("./%s/app/testpmd -c f -n 4 -- -i --txqflags=0" %
> self.target, "testpmd> ", 120)
> > +        out = self.dut.send_expect("create bonded device 3 0", "testpmd> ",
> 30)
> > +        bond_device_id = int(re.search("port \d+", out).group().split(" ")[-
> 1].strip())
> > +        self.verify(bond_device_id > 1, "not enought port for bonded
> > + test")
> 
> Please add some space line between different blocks. And add one
> comment line to describe the code block.
> It will more readable and easier for later maintenance.
Yes, I will updte next version.
> > +        self.dut.send_expect("add bonding slave 0 %d" % bond_device_id,
> "testpmd>", 30)
> > +        self.dut.send_expect("add bonding slave 1 %d" % bond_device_id,
> "testpmd>", 30)
> > +        out = self.dut.send_expect("get_hash_global_config 0", "testpmd>")
> > +        slave0_hash_function = re.search("Hash function is .+",
> out).group().split(" ")[-1].strip()
> > +        out = self.dut.send_expect("get_hash_global_config 1", "testpmd>")
> > +        slave1_hash_function = re.search("Hash function is .+",
> out).group().split(" ")[-1].strip()
> > +        self.verify(slave0_hash_function == slave1_hash_function, "default
> hash function not match")
> > +        new_hash_function = ""
> > +        for hash_function in ["toeplitz", "simple_xor"]:
> > +            if slave0_hash_function[-3:].lower() != hash_function[-3:]:
> > +                new_hash_function = hash_function
> > +        self.dut.send_expect("set_hash_global_config 0 %s ipv4-other
> enable" % new_hash_function, "testpmd>")
> > +        out = self.dut.send_expect("get_hash_global_config 0", "testpmd>")
> > +        slave0_new_hash_function = re.search("Hash function is .+",
> out).group().split(" ")[-1].strip()
> > +        out = self.dut.send_expect("get_hash_global_config 1", "testpmd>")
> > +        slave1_new_hash_function = re.search("Hash function is .+",
> out).group().split(" ")[-1].strip()
> > +        self.verify(slave0_new_hash_function == slave1_new_hash_function,
> "bond slave auto sync hash function failed")
> > +        self.verify(slave0_new_hash_function[-3:].lower() ==
> new_hash_function[-3:], "changed slave hash function failed")
> > +        self.dut.send_expect("remove bonding slave 0 %d" %
> bond_device_id, "testpmd>", 30)
> > +        self.dut.send_expect("remove bonding slave 1 %d" %
> bond_device_id, "testpmd>", 30)
> > +        self.dut.send_expect("quit","# ", 30)
> >       def tear_down(self):
> >           """
> >           Run after each test case.