<div dir="ltr">Hi Lijuan,<br><div><br></div><div>Sorry, what you suggested makes sense. The V2 I send was some changes that will make the code cleaner by combining some duplicated code in the V1. To solve this issue, I will submit a patch that combines both of these versions. </div><div><br></div><div>Thanks</div><div>David</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jul 23, 2020 at 10:01 PM Tu, Lijuan <<a href="mailto:lijuan.tu@intel.com" target="_blank">lijuan.tu@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi David,<br>
<br>
V2 should be also based on DTS remote master, but not your local code.<br>
<br>
When V1 are not merged, we submit V2, V2 should cover V1+ fix comments based on DTS remote code, maintainer will ignore V1 and merge V2. It's not make sense to merge patches with obvious defect. <br>
<br>
> -----Original Message-----<br>
> From: dts <<a href="mailto:dts-bounces@dpdk.org" target="_blank">dts-bounces@dpdk.org</a>> On Behalf Of David Liu<br>
> Sent: 2020年7月16日 4:09<br>
> To: <a href="mailto:dts@dpdk.org" target="_blank">dts@dpdk.org</a><br>
> Cc: <a href="mailto:lylavoie@iol.unh.edu" target="_blank">lylavoie@iol.unh.edu</a>; David Liu <<a href="mailto:dliu@iol.unh.edu" target="_blank">dliu@iol.unh.edu</a>><br>
> Subject: [dts] [PATCH v2]Add RSS Key Update Feature<br>
> <br>
> Merge duplicated code. Add flag in send packet and verify result to identify<br>
> symmetric used.<br>
> <br>
> Signed-off-by: David Liu <<a href="mailto:dliu@iol.unh.edu" target="_blank">dliu@iol.unh.edu</a>><br>
> ---<br>
> tests/TestSuite_rss_key_update.py | 183 ++++++++----------------------<br>
> 1 file changed, 46 insertions(+), 137 deletions(-)<br>
> <br>
> diff --git a/tests/TestSuite_rss_key_update.py<br>
> b/tests/TestSuite_rss_key_update.py<br>
> index 0985f30..adb44f1 100644<br>
> --- a/tests/TestSuite_rss_key_update.py<br>
> +++ b/tests/TestSuite_rss_key_update.py<br>
> @@ -39,7 +39,6 @@ Test the support of RSS Key Update by Poll Mode Drivers.<br>
> <br>
> import time<br>
> import re<br>
> -import packet<br>
> import random<br>
> import utils<br>
> <br>
> @@ -63,85 +62,37 @@ iptypes = {'ipv4-sctp': 'sctp',<br>
> <br>
> class TestRssKeyUpdate(TestCase):<br>
> <br>
> - def send_packet(self, itf, tran_type):<br>
> + def send_packet(self, itf, tran_type, symmetric):<br>
> """<br>
> Sends packets.<br>
> """<br>
> + packet_list = {<br>
> + 'ipv4-sctp': 'IP(src="192.168.0.%d",<br>
> dst="192.168.0.%d")/SCTP(sport=1024,dport=1024,tag=1)',<br>
> + 'ipv4-other': 'IP(src="192.168.0.%d", dst="192.168.0.%d")',<br>
> + 'ipv4-frag': 'IP(src="192.168.0.%d",<br>
> dst="192.168.0.%d",frag=1,flags="MF")',<br>
> + 'ipv4-udp': 'IP(src="192.168.0.%d",<br>
> dst="192.168.0.%d")/UDP(sport=1024,dport=1024)',<br>
> + 'ipv4-tcp': 'IP(src="192.168.0.%d",<br>
> dst="192.168.0.%d")/TCP(sport=1024,dport=1024)',<br>
> + 'ipv6-other':'IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d")',<br>
> + 'ipv6-sctp': 'IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d", nh=132)/SCTP(sport=1024,dport=1024,tag=1)',<br>
> + 'ipv6-udp': 'IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d")/UDP(sport=1024,dport=1024)',<br>
> + 'ipv6-tcp': 'IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d")/TCP(sport=1024,dport=1024)',<br>
> + 'ipv6-frag': 'IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d",nh=44)/IPv6ExtHdrFragment()'<br>
> + }<br>
> +<br>
> received_pkts = []<br>
> self.tester.scapy_foreground()<br>
> self.dut.send_expect("start", "testpmd>")<br>
> mac = self.dut.get_mac_address(0)<br>
> <br>
> # send packet with different source and dest ip<br>
> - if tran_type == "ipv4-other":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IP(src="192.168.0.%d", dst="192.168.0.%d")],<br>
> iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> - elif tran_type == "ipv4-tcp":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IP(src="192.168.0.%d",<br>
> dst="192.168.0.%d")/TCP(sport=1024,dport=1024)], iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> - elif tran_type == "ipv4-udp":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IP(src="192.168.0.%d",<br>
> dst="192.168.0.%d")/UDP(sport=1024,dport=1024)], iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> - elif tran_type == "ipv4-sctp":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IP(src="192.168.0.%d",<br>
> dst="192.168.0.%d")/SCTP(sport=1024,dport=1024,tag=1)], iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> - elif tran_type == "ipv4-frag":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IP(src="192.168.0.%d",<br>
> dst="192.168.0.%d",frag=1,flags="MF")], iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> -<br>
> - elif tran_type == "ipv6-other":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d")], iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> - elif tran_type == "ipv6-tcp":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d")/TCP(sport=1024,dport=1024)], iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> - elif tran_type == "ipv6-udp":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d")/UDP(sport=1024,dport=1024)], iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> - elif tran_type == "ipv6-sctp":<br>
> + if tran_type in packet_list.keys():<br>
> + packet_temp = r'sendp([Ether(dst="%s",<br>
> + src=get_if_hwaddr("%s"))/%s], iface="%s")' % (mac, itf,<br>
> + packet_list[tran_type], itf)<br>
> for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d", nh=132)/SCTP(sport=1024,dport=1024,tag=1)],<br>
> iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> - self.tester.scapy_append(packet)<br>
> - self.tester.scapy_execute()<br>
> - time.sleep(.5)<br>
> - elif tran_type == "ipv6-frag":<br>
> - for i in range(10):<br>
> - packet = r'sendp([Ether(dst="%s",<br>
> src=get_if_hwaddr("%s"))/IPv6(src="3ffe:2501:200:1fff::%d",<br>
> dst="3ffe:2501:200:3::%d",nh=44)/IPv6ExtHdrFragment()], iface="%s")' % (<br>
> - mac, itf, i + 1, i + 2, itf)<br>
> + packet = packet_temp % (i + 1, i + 2)<br>
> self.tester.scapy_append(packet)<br>
> + if symmetric:<br>
> + packet2 = packet_list[tran_type] % (mac, itf, i + 2, i + 1, itf)<br>
> + self.tester.scapy_append(packet2)<br>
> self.tester.scapy_execute()<br>
> time.sleep(.5)<br>
> else:<br>
> @@ -154,102 +105,64 @@ class TestRssKeyUpdate(TestCase):<br>
> # collect the hash result and the queue id<br>
> for line in lines:<br>
> line = line.strip()<br>
> - if len(line) != 0 and line.strip().startswith("port "):<br>
> + if len(line) != 0 and line.startswith("port "):<br>
> reta_line = {}<br>
> - rexp = r"port (\d)/queue (\d{1,2}): received (\d) packets"<br>
> - m = re.match(rexp, line.strip())<br>
> + rexp = r"port (\d+)/queue (\d+): received (\d+) packets"<br>
> + m = re.match(rexp, line)<br>
> if m:<br>
> reta_line["port"] = m.group(1)<br>
> reta_line["queue"] = m.group(2)<br>
> <br>
> - elif len(line) != 0 and line.startswith(("src=",)):<br>
> + elif len(line) != 0 and line.startswith("src="):<br>
> if "RSS hash" not in line:<br>
> continue<br>
> for item in line.split("-"):<br>
> item = item.strip()<br>
> - if(item.startswith("RSS hash")):<br>
> + if item.startswith("RSS hash"):<br>
> name, value = item.split("=", 1)<br>
> <br>
> reta_line[name.strip()] = value.strip()<br>
> received_pkts.append(reta_line)<br>
> <br>
> - return(self.verifyResult(received_pkts))<br>
> + return(self.verifyResult(received_pkts, symmetric))<br>
> <br>
> - def verifyResult(self, reta_lines):<br>
> + def verifyResult(self, reta_lines, symmetric):<br>
> """<br>
> Verify whether or not the result passes.<br>
> """<br>
> -<br>
> - global reta_num<br>
> result = []<br>
> key_id = {}<br>
> self.verify(len(reta_lines) > 0, 'No packet received!')<br>
> self.result_table_create(<br>
> ['packet index', 'hash value', 'hash index', 'queue id', 'actual queue id',<br>
> 'pass '])<br>
> <br>
> - i = 0<br>
> -<br>
> - for tmp_reta_line in reta_lines:<br>
> + for i, tmp_reta_line in enumerate(reta_lines):<br>
> status = "false"<br>
> # compute the hash result of five tuple into the 7 LSBs value.<br>
> hash_index = int(tmp_reta_line["RSS hash"], 16) % reta_num<br>
> - print(reta_entries[hash_index], tmp_reta_line)<br>
> if(reta_entries[hash_index] == int(tmp_reta_line["queue"])):<br>
> status = "true"<br>
> result.insert(i, 0)<br>
> + if symmetric:<br>
> + if(i % 2 == 1):<br>
> + if(pre_RSS_hash == tmp_reta_line["RSS hash"]):<br>
> + status = "true"<br>
> + result.insert(len(reta_lines) + (i - 1) // 2, 0)<br>
> + else:<br>
> + status = "fail"<br>
> + result.insert(len(reta_lines) + (i - 1) // 2, 1)<br>
> + pre_RSS_hash = tmp_reta_line["RSS hash"]<br>
> else:<br>
> status = "fail"<br>
> result.insert(i, 1)<br>
> self.result_table_add(<br>
> [i, tmp_reta_line["RSS hash"], hash_index, reta_entries[hash_index],<br>
> tmp_reta_line["queue"], status])<br>
> - i = i + 1<br>
> - key_id[tmp_reta_line["RSS hash"]]=reta_entries[hash_index]<br>
> + key_id[tmp_reta_line["RSS hash"]] =<br>
> + reta_entries[hash_index]<br>
> <br>
> self.result_table_print()<br>
> self.verify(sum(result) == 0, "the reta update function failed!")<br>
> return key_id<br>
> <br>
> - def verifyResult_symmetric(self, reta_lines):<br>
> - """<br>
> - Verify whether or not the result passes.<br>
> - """<br>
> -<br>
> - global reta_num<br>
> - result = []<br>
> - key_id = {}<br>
> - self.verify(len(reta_lines) > 0, 'No packet received!')<br>
> - self.result_table_create(<br>
> - ['packet index', 'RSS hash', 'hash index', 'queue id', 'actual queue id', 'pass<br>
> '])<br>
> -<br>
> - i = 0<br>
> - for tmp_reta_line in reta_lines:<br>
> - status = "false"<br>
> - # compute the hash result of five tuple into the 7 LSBs value.<br>
> - hash_index = int(tmp_reta_line["RSS hash"], 16) % reta_num<br>
> - if(reta_entries[hash_index] == int(tmp_reta_line["queue"])):<br>
> - status = "true"<br>
> - result.insert(i, 0)<br>
> - if(i % 2 == 1):<br>
> - if(pre_RSS_hash == tmp_reta_line["RSS hash"]):<br>
> - status = "true"<br>
> - result.insert(len(reta_lines) + (i - 1) // 2, 0)<br>
> - else:<br>
> - status = "fail"<br>
> - result.insert(len(reta_lines) + (i - 1) // 2, 1)<br>
> - pre_RSS_hash = tmp_reta_line["RSS hash"]<br>
> - else:<br>
> - status = "fail"<br>
> - result.insert(i, 1)<br>
> - self.result_table_add(<br>
> - [i, tmp_reta_line["RSS hash"], hash_index, reta_entries[hash_index],<br>
> tmp_reta_line["queue"], status])<br>
> - i = i + 1<br>
> - key_id[tmp_reta_line["RSS hash"]]=reta_entries[hash_index]<br>
> -<br>
> - self.result_table_print()<br>
> - self.verify(<br>
> - sum(result) == 0, "the symmetric RSS hash function failed!")<br>
> - return key_id<br>
> -<br>
> def set_up_all(self):<br>
> """<br>
> Run at the start of each test suite.<br>
> @@ -282,7 +195,7 @@ class TestRssKeyUpdate(TestCase):<br>
> elif self.nic in ["redrockcanyou", "atwood", "boulderrapid"]:<br>
> reta_num = 128<br>
> else:<br>
> - self.verify(False, f"NIC Unsupported:{self.nic}")<br>
> + self.verify(False, f"NIC Unsupported: {self.nic}")<br>
> <br>
> cores = self.dut.get_core_list("all")<br>
> self.coremask = utils.create_mask(cores) @@ -300,8 +213,6 @@ class<br>
> TestRssKeyUpdate(TestCase):<br>
> dutPorts = self.dut.get_ports(self.nic)<br>
> localPort = self.tester.get_local_port(dutPorts[0])<br>
> self.itf = self.tester.get_interface(localPort)<br>
> - global reta_num<br>
> - global iptypes<br>
> <br>
> self.dut.kill_all()<br>
> <br>
> @@ -326,12 +237,12 @@ class TestRssKeyUpdate(TestCase):<br>
> reta_entries.insert(i, random.randint(0, queue - 1))<br>
> self.dut.send_expect(f"port config 0 rss reta ({i},{reta_entries[i]})",<br>
> "testpmd> ")<br>
> <br>
> - ori_output = self.send_packet(self.itf, iptype)<br>
> + ori_output = self.send_packet(self.itf, iptype, False)<br>
> <br>
> self.dut.send_expect("show port 0 rss-hash key", "testpmd> ")<br>
> self.dut.send_expect(f"port config 0 rss-hash-key {iptype}<br>
> 4439796BB54C50f3B675EF5B124F9F30B8A2C0FFFFDC4D02A08C9B334FF64A4C<br>
> 05C6FA343958D855FFF9583AE138C92E81150FFF", "testpmd> ")<br>
> <br>
> - new_output = self.send_packet(self.itf, iptype)<br>
> + new_output = self.send_packet(self.itf, iptype, False)<br>
> <br>
> self.verify(ori_output != new_output, "before and after results are the<br>
> same, hash key configuration failed!")<br>
> <br>
> @@ -356,16 +267,15 @@ class TestRssKeyUpdate(TestCase):<br>
> reta_entries.insert(i, random.randint(0, queue - 1))<br>
> self.dut.send_expect(f"port config 0 rss reta ({i},{reta_entries[i]})",<br>
> "testpmd> ")<br>
> <br>
> - ori_output = self.send_packet(self.itf, iptype)<br>
> + ori_output = self.send_packet(self.itf, iptype, True)<br>
> <br>
> out = self.dut.send_expect("show port 0 rss-hash key", "testpmd> ")<br>
> self.verify("rss disable" not in out, "rss is disable!")<br>
> self.dut.send_expect(f"port config 0 rss-hash-key {iptype}<br>
> 4439796BB54C50f3B675EF5B124F9F30B8A2C0FFFFDC4D02A08C9B334FF64A4C<br>
> 05C6FA343958D855FFF9583AE138C92E81150FFF", "testpmd> ")<br>
> <br>
> - new_output = self.send_packet(self.itf, iptype)<br>
> + new_output = self.send_packet(self.itf, iptype, True)<br>
> <br>
> self.verify(ori_output != new_output, "before and after results are the<br>
> same, hash key configuration failed!")<br>
> - self.dut.send_expect("quit", "# ", 30)<br>
> <br>
> def test_set_hash_key_short_long(self):<br>
> <br>
> @@ -384,26 +294,25 @@ class TestRssKeyUpdate(TestCase):<br>
> out = self.dut.send_expect("show port info all", "testpmd> ", 120)<br>
> self.verify(f"Hash key size in bytes: {nic_rss_key_size[self.nic]}" in out, "not<br>
> expected hash key size!")<br>
> <br>
> - test_keies = {<br>
> + test_keys = {<br>
> <br>
> "4439796BB54C50f3B675EF5B124F9F30B8A2C0FFFFDC4D02A08C9B334FF64A4<br>
> C05C6FA343958D855FFF9583AE138C92E81150FFFFF": "longer",<br>
> <br>
> "4439796BB54C50f3B675EF5B124F9F30B8A2C0DC4D02A08C9B334FF64A4C05C<br>
> 6FA343958D855FFF9583AE138C92E81150FFF": "shorter",<br>
> }<br>
> <br>
> # config key length longer/shorter than 104 hexa-decimal numbers<br>
> - for key, error in test_keies.items():<br>
> + for key, error in test_keys.items():<br>
> out = self.dut.send_expect(f"port config 0 rss-hash-key ipv4-udp {key}",<br>
> "testpmd> ")<br>
> self.verify("invalid" in out, f"Try to set hash key {error} than 104 hexa-<br>
> decimal numbers!")<br>
> <br>
> # config ket length same as 104 hex-decimal numbers and keep the config<br>
> key =<br>
> "4439796BB54C50f3B675EF5B124F9F30B8A2C0FFFFDC4D02A08C9B334FF64A4<br>
> C05C6FA343958D855FFF9583AE138C92E81150FFF"<br>
> out = self.dut.send_expect(f"port config 0 rss-hash-key ipv4-udp {key}",<br>
> "testpmd> ")<br>
> - self.dut.send_expect("quit", "# ", 30)<br>
> <br>
> def tear_down(self):<br>
> """<br>
> Run after each test case.<br>
> """<br>
> - self.dut.send_expect("quit", "# ", 30)<br>
> + self.pmdout.quit()<br>
> <br>
> def tear_down_all(self):<br>
> """<br>
> --<br>
> 2.17.1<br>
<br>
</blockquote></div>