From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 9EDED1C00 for ; Fri, 12 May 2017 11:06:22 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 May 2017 02:06:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,328,1491289200"; d="scan'208";a="85952639" Received: from stv-crb-56.sh.intel.com (HELO [10.239.128.116]) ([10.239.128.116]) by orsmga002.jf.intel.com with ESMTP; 12 May 2017 02:06:20 -0700 Message-ID: <59157823.2030805@intel.com> Date: Fri, 12 May 2017 16:53:55 +0800 From: "Liu, Yong" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: "xu,huilong" , dts@dpdk.org References: <1494575072-44798-1-git-send-email-huilongx.xu@intel.com> <1494575072-44798-11-git-send-email-huilongx.xu@intel.com> In-Reply-To: <1494575072-44798-11-git-send-email-huilongx.xu@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dts] [PATCH 2/2] add link bond slave dynamic config test case X-BeenThere: dts@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: test suite reviews and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 May 2017 09:06:23 -0000 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 > --- > 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. > "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? > > 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. > + 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.