Re: [dts] [PATCH V1 2/2] add dyanmic config bonding rss hash test case

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

> -----Original Message-----
> From: Liu, Yong
> Sent: Wednesday, March 01, 2017 9:31 PM
> To: Xu, HuilongX; dts@dpdk.org
> Cc: Xu, HuilongX
> Subject: RE: [dts] [PATCH V1 2/2] add dyanmic config bonding rss hash test
> case
> 
> Hi Huilong,
> 
> Some comments below.
> 
> Thanks,
> Marvin
> 
> > -----Original Message-----
> > From: dts [mailto:dts-bounces@dpdk.org] On Behalf Of xu,huilong
> > Sent: Wednesday, March 01, 2017 5:25 PM
> > To: dts@dpdk.org
> > Cc: Xu, HuilongX <huilongx.xu@intel.com>
> > Subject: [dts] [PATCH V1 2/2] add dyanmic config bonding rss hash test
> > case
> >
> > Signed-off-by: xu,huilong <huilongx.xu@intel.com>
> > ---
> >  tests/TestSuite_pmdrss_hash.py | 35
> > ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/TestSuite_pmdrss_hash.py
> > b/tests/TestSuite_pmdrss_hash.py index 78ae34d..5ab8b6b 100644
> > --- a/tests/TestSuite_pmdrss_hash.py
> > +++ b/tests/TestSuite_pmdrss_hash.py
> > @@ -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")
> >
> >      def set_up(self):
> >          """
> > @@ -650,6 +650,39 @@ class TestPmdrssHash(TestCase):
> >                  self.send_packet_symmetric(itf, iptype)
> >
> >              self.dut.send_expect("quit", "# ", 30)
> 
> Need blank line here.
Thanks, I will send V2 patch and fix it
> 
> 
> > +    def test_dyanmic_rss_bond_config(self):
> 
> Typo, should be dynamic.
Thanks  I will send V2 patch and fix it
> 
> 
> > +        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")
> 
> This criteria has been validated in set_up_all function, no need here.
Yes, I will remove it in V2 patch
> 
> > +
> > +        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")
> > +
> Not sure set and get is enough for validation, hash function not checked here.
This case only check update slave0 hash function, and slave1 will auto sync hash function.
So there are two check point. 
First point: salv0 and slav1 hash function must same
Second point: slav0 hash function update successful.
> 
> > +        self.dut.send_expect("quit","# ", 30)
> > +
> >
> >      def tear_down(self):
> >          """
> > --
> > 1.9.3