test suite reviews and discussions
 help / color / mirror / Atom feed
From: "Liu, Yong" <yong.liu@intel.com>
To: "Xu, GangX" <gangx.xu@intel.com>, "dts@dpdk.org" <dts@dpdk.org>
Cc: "Xu, GangX" <gangx.xu@intel.com>
Subject: Re: [dts] [PATCH V2] fix ipfrag failed on freebsd
Date: Thu, 7 Sep 2017 02:31:55 +0000	[thread overview]
Message-ID: <86228AFD5BCD8E4EBFD2B90117B5E81E62E9DAB7@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1504688779-5677-1-git-send-email-gangx.xu@intel.com>

Gang, one comment below. I knowing that you are fixing all PEP issues in the suite. 
But for some one line commands, it will be more readable to keep its original format.
You can skip this error by adding "--ignore E501" between pep8 command.

Thanks,
Marvin

> -----Original Message-----
> From: dts [mailto:dts-bounces@dpdk.org] On Behalf Of xu,gang
> Sent: Wednesday, September 06, 2017 5:06 PM
> To: dts@dpdk.org
> Cc: Xu, GangX <gangx.xu@intel.com>
> Subject: [dts] [PATCH V2] fix ipfrag failed on freebsd
> 
> modify command can;t work on freebsd, so do it on tester
> 
> Signed-off-by: xu,gang <gangx.xu@intel.com>
> ---
>  tests/TestSuite_ipfrag.py | 125 +++++++++++++++++++++++++++++++----------
> -----
>  1 file changed, 86 insertions(+), 39 deletions(-)
> 
> diff --git a/tests/TestSuite_ipfrag.py b/tests/TestSuite_ipfrag.py
> index f23dbe1..c80594a 100644
> --- a/tests/TestSuite_ipfrag.py
> +++ b/tests/TestSuite_ipfrag.py
> @@ -84,7 +84,6 @@ class TestIpfrag(TestCase):
> 
>          # Based on h/w type, choose how many ports to use
>          ports = self.dut.get_ports()
> -        print ports
> 
>          # Verify that enough ports are available
>          self.verify(len(ports) >= 2, "Insufficient ports for testing")
> @@ -109,16 +108,28 @@ l3fwd_ipv4_route_array[] = {\\\n"
>              rtLpmTbl[idx] = pat.sub(self.portRepl, rtLpmTbl[idx])
>              lpmStr_ipv4 = lpmStr_ipv4 + ' ' * 4 + rtLpmTbl[idx] + ",\\\n"
>          lpmStr_ipv4 = lpmStr_ipv4 + "};"
> -        print lpmStr_ipv4
>          lpmStr_ipv6 = "static struct l3fwd_ipv6_route
> l3fwd_ipv6_route_array[] = {\\\n"
>          rtLpmTbl = list(lpm_table_ipv6)
>          for idx in range(len(rtLpmTbl)):
>              rtLpmTbl[idx] = pat.sub(self.portRepl, rtLpmTbl[idx])
>              lpmStr_ipv6 = lpmStr_ipv6 + ' ' * 4 + rtLpmTbl[idx] + ",\\\n"
>          lpmStr_ipv6 = lpmStr_ipv6 + "};"
> -        print lpmStr_ipv6
> -        self.dut.send_expect(r"sed -i
> '/l3fwd_ipv4_route_array\[\].*{/,/^\}\;/c\\%s'
> examples/ip_fragmentation/main.c" % lpmStr_ipv4, "# ")
> -        self.dut.send_expect(r"sed -i
> '/l3fwd_ipv6_route_array\[\].*{/,/^\}\;/c\\%s'
> examples/ip_fragmentation/main.c" % lpmStr_ipv6, "# ")
> +        if self.dut.get_os_type() == 'freebsd':
> +            self.dut.session.copy_file_from(
> +                '/root/dpdk/examples/ip_fragmentation/main.c', '/tmp')
> +            self.tester.send_expect(
> +                r"sed -i '/l3fwd_ipv4_route_array\[\].*{/,/^\}\;/c\\%s'
> /tmp/main.c" % lpmStr_ipv4, "# ")
> +            self.tester.send_expect(
> +                r"sed -i '/l3fwd_ipv6_route_array\[\].*{/,/^\}\;/c\\%s'
> /tmp/main.c" % lpmStr_ipv6, "# ")
> +            self.dut.session.copy_file_to(
> +                '/tmp/main.c', '/root/dpdk/examples/ip_fragmentation/')
> +

Gang, why not just do it in DUT?

> +        else:
> +            self.dut.send_expect(
> +                r"sed -i '/l3fwd_ipv4_route_array\[\].*{/,/^\}\;/c\\%s'
> examples/ip_fragmentation/main.c" % lpmStr_ipv4, "# ")
> +            self.dut.send_expect(
> +                r"sed -i '/l3fwd_ipv6_route_array\[\].*{/,/^\}\;/c\\%s'
> examples/ip_fragmentation/main.c" % lpmStr_ipv6, "# ")
> +
>          # make application
>          out = self.dut.build_dpdk_apps("examples/ip_fragmentation")
>          self.verify("Error" not in out, "compilation error 1")
> @@ -148,7 +159,8 @@ l3fwd_ipv4_route_array[] = {\\\n"
>              # simulate to set TG properties
>              if flag == 'frag':
>                  # do fragment, each packet max length 1518 - 18 - 20 =
> 1480
> -                expPkts = (size - HEADER_SIZE['eth'] - HEADER_SIZE['ip'])
> / 1480
> +                expPkts = (
> +                    size - HEADER_SIZE['eth'] - HEADER_SIZE['ip']) / 1480
>                  if (size - HEADER_SIZE['eth'] - HEADER_SIZE['ip']) % 1480:
>                      expPkts += 1
>                  val = 0
> @@ -165,12 +177,15 @@ l3fwd_ipv4_route_array[] = {\\\n"
>                  pkt_size = pkt_sizes[pkt_sizes.index(size) + times]
>                  pkt = Packet(pkt_type='UDP', pkt_len=pkt_size)
>                  pkt.config_layer('ether', {'dst': self.dmac})
> -                pkt.config_layer('ipv4', {'dst': '100.10.0.1', 'src':
> '1.2.3.4', 'flags': val})
> +                pkt.config_layer(
> +                    'ipv4', {'dst': '100.10.0.1', 'src': '1.2.3.4',
> 'flags': val})
>                  pkt.send_pkt(tx_port=self.txItf)
> 
> -            # verify normal packet just by number, verify fragment packet
> by all elements
> +            # verify normal packet just by number, verify fragment packet
> by
> +            # all elements
>              pkts = load_sniff_packets(inst)
> -            self.verify(len(pkts) == expPkts, "Failed on forward packet
> size " + str(size))
> +            self.verify(
> +                len(pkts) == expPkts, "Failed on forward packet size " +
> str(size))
>              if flag == 'frag':
>                  idx = 1
>                  for pkt in pkts:
> @@ -178,19 +193,23 @@ l3fwd_ipv4_route_array[] = {\\\n"
>                      pkt_id = pkt.strip_element_layer3("id")
>                      if idx == 1:
>                          prev_idx = pkt_id
> -                    self.verify(prev_idx == pkt_id, "Fragmented packets
> index not match")
> +                    self.verify(
> +                        prev_idx == pkt_id, "Fragmented packets index not
> match")
>                      prev_idx = pkt_id
> 
>                      # last flags should be 0
>                      flags = pkt.strip_element_layer3("flags")
>                      if idx == expPkts:
> -                        self.verify(flags == 0, "Fragmented last packet
> flags not match")
> +                        self.verify(
> +                            flags == 0, "Fragmented last packet flags not
> match")
>                      else:
> -                        self.verify(flags == 1, "Fragmented packets flags
> not match")
> +                        self.verify(
> +                            flags == 1, "Fragmented packets flags not
> match")
> 
>                      # fragment offset should be correct
>                      frag = pkt.strip_element_layer3("frag")
> -                    self.verify((frag == ((idx - 1) * 185)), "Fragment
> packet frag not match")
> +                    self.verify(
> +                        (frag == ((idx - 1) * 185)), "Fragment packet
> frag not match")
>                      idx += 1
> 
>      def functional_check_ipv6(self, pkt_sizes, burst=1, flag=None,
> funtion=None):
> @@ -200,8 +219,10 @@ l3fwd_ipv4_route_array[] = {\\\n"
>          for size in pkt_sizes[::burst]:
>              # simulate to set TG properties
>              if flag == 'frag':
> -                # each packet max len: 1518 - 18 (eth) - 40 (ipv6) - 8
> (ipv6 ext hdr) = 1452
> -                expPkts = (size - HEADER_SIZE['eth'] -
> HEADER_SIZE['ipv6']) / 1452
> +                # each packet max len: 1518 - 18 (eth) - 40 (ipv6) - 8
> (ipv6
> +                # ext hdr) = 1452
> +                expPkts = (
> +                    size - HEADER_SIZE['eth'] - HEADER_SIZE['ipv6']) /
> 1452
>                  if (size - HEADER_SIZE['eth'] - HEADER_SIZE['ipv6']) %
> 1452:
>                      expPkts += 1
>                  val = 0
> @@ -215,12 +236,15 @@ l3fwd_ipv4_route_array[] = {\\\n"
>                  pkt_size = pkt_sizes[pkt_sizes.index(size) + times]
>                  pkt = Packet(pkt_type='IPv6_UDP', pkt_len=pkt_size)
>                  pkt.config_layer('ether', {'dst': self.dmac})
> -                pkt.config_layer('ipv6', {'dst':
> '101:101:101:101:101:101:101:101', 'src':
> 'ee80:ee80:ee80:ee80:ee80:ee80:ee80:ee80'})
> +                pkt.config_layer(
> +                    'ipv6', {'dst': '101:101:101:101:101:101:101:101',
> 'src': 'ee80:ee80:ee80:ee80:ee80:ee80:ee80:ee80'})
>                  pkt.send_pkt(tx_port=self.txItf)
> 
> -            # verify normal packet just by number, verify fragment packet
> by all elements
> +            # verify normal packet just by number, verify fragment packet
> by
> +            # all elements
>              pkts = load_sniff_packets(inst)
> -            self.verify(len(pkts) == expPkts, "Failed on forward packet
> size " + str(size))
> +            self.verify(
> +                len(pkts) == expPkts, "Failed on forward packet size " +
> str(size))
>              if flag == 'frag':
>                  idx = 1
>                  for pkt in pkts:
> @@ -228,27 +252,33 @@ l3fwd_ipv4_route_array[] = {\\\n"
>                      pkt_id = pkt.strip_element_layer4("id")
>                      if idx == 1:
>                          prev_idx = pkt_id
> -                    self.verify(prev_idx == pkt_id, "Fragmented packets
> index not match")
> +                    self.verify(
> +                        prev_idx == pkt_id, "Fragmented packets index not
> match")
>                      prev_idx = pkt_id
> 
>                      # last flags should be 0
>                      flags = pkt.strip_element_layer4("m")
>                      if idx == expPkts:
> -                        self.verify(flags == 0, "Fragmented last packet
> flags not match")
> +                        self.verify(
> +                            flags == 0, "Fragmented last packet flags not
> match")
>                      else:
> -                        self.verify(flags == 1, "Fragmented packets flags
> not match")
> +                        self.verify(
> +                            flags == 1, "Fragmented packets flags not
> match")
> 
>                      # fragment offset should be correct
>                      frag = pkt.strip_element_layer4("offset")
> -                    self.verify((frag == int((idx - 1) * 181.5)),
> "Fragment packet frag not match")
> +                    self.verify(
> +                        (frag == int((idx - 1) * 181.5)), "Fragment
> packet frag not match")
>                      idx += 1
> 
>      def set_up(self):
>          """
>          Run before each test case.
>          """
> -        self.tester.send_expect("ifconfig %s mtu 9200" %
> self.tester.get_interface(self.tester.get_local_port(P0)), "#")
> -        self.tester.send_expect("ifconfig %s mtu 9200" %
> self.tester.get_interface(self.tester.get_local_port(P1)), "#")
> +        self.tester.send_expect("ifconfig %s mtu 9200" %
> +
> self.tester.get_interface(self.tester.get_local_port(P0)), "#")
> +        self.tester.send_expect("ifconfig %s mtu 9200" %
> +
> self.tester.get_interface(self.tester.get_local_port(P1)), "#")
> 
>      def test_ipfrag_normalfwd(self):
>          """
> @@ -290,7 +320,8 @@ l3fwd_ipv4_route_array[] = {\\\n"
>          Pct = dict()
> 
>          if int(lcore[0]) == 1:
> -            core_mask = utils.create_mask(self.dut.get_core_list(lcore,
> socket=self.ports_socket))
> +            core_mask = utils.create_mask(
> +                self.dut.get_core_list(lcore, socket=self.ports_socket))
>          else:
>              core_mask = utils.create_mask(self.dut.get_core_list(lcore))
> 
> @@ -302,29 +333,42 @@ l3fwd_ipv4_route_array[] = {\\\n"
>          result = [2, lcore, num_pthreads]
>          for size in size_list:
>              dmac = self.dut.get_mac_address(P0)
> -            flows = ['Ether(dst="%s")/IP(src="1.2.3.4", dst="100.10.0.1",
> flags=0)/("X"*%d)' % (dmac, size - 38),
> -                     'Ether(dst="%s")/IP(src="1.2.3.4", dst="100.20.0.1",
> flags=0)/("X"*%d)' % (dmac, size - 38),
> -
> 'Ether(dst="%s")/IPv6(dst="101:101:101:101:101:101:101:101",src="ee80:ee80
> :ee80:ee80:ee80:ee80:ee80:ee80")/Raw(load="X"*%d)' % (dmac, size - 58),
> +            flows = [
> +                'Ether(dst="%s")/IP(src="1.2.3.4", dst="100.10.0.1",
> flags=0)/("X"*%d)' % (
> +                    dmac, size - 38),
> +                     'Ether(dst="%s")/IP(src="1.2.3.4", dst="100.20.0.1",
> flags=0)/("X"*%d)' % (
> +                         dmac, size - 38),
> +
> 'Ether(dst="%s")/IPv6(dst="101:101:101:101:101:101:101:101",src="ee80:ee80
> :ee80:ee80:ee80:ee80:ee80:ee80")/Raw(load="X"*%d)' % (
> +                         dmac, size - 58),
> 
> 'Ether(dst="%s")/IPv6(dst="201:101:101:101:101:101:101:101",src="ee80:ee80
> :ee80:ee80:ee80:ee80:ee80:ee80")/Raw(load="X"*%d)' % (dmac, size - 58)]
> -            self.tester.scapy_append('wrpcap("test1.pcap", [%s])' %
> string.join(flows, ','))
> +            self.tester.scapy_append(
> +                'wrpcap("test1.pcap", [%s])' % string.join(flows, ','))
> 
>              # reserved for rx/tx bidirection test
>              dmac = self.dut.get_mac_address(P1)
> -            flows = ['Ether(dst="%s")/IP(src="1.2.3.4", dst="100.30.0.1",
> flags=0)/("X"*%d)' % (dmac, size - 38),
> -                     'Ether(dst="%s")/IP(src="1.2.3.4", dst="100.40.0.1",
> flags=0)/("X"*%d)' % (dmac, size - 38),
> -
> 'Ether(dst="%s")/IPv6(dst="301:101:101:101:101:101:101:101",src="ee80:ee80
> :ee80:ee80:ee80:ee80:ee80:ee80")/Raw(load="X"*%d)' % (dmac, size - 58),
> +            flows = [
> +                'Ether(dst="%s")/IP(src="1.2.3.4", dst="100.30.0.1",
> flags=0)/("X"*%d)' % (
> +                    dmac, size - 38),
> +                     'Ether(dst="%s")/IP(src="1.2.3.4", dst="100.40.0.1",
> flags=0)/("X"*%d)' % (
> +                         dmac, size - 38),
> +
> 'Ether(dst="%s")/IPv6(dst="301:101:101:101:101:101:101:101",src="ee80:ee80
> :ee80:ee80:ee80:ee80:ee80:ee80")/Raw(load="X"*%d)' % (
> +                         dmac, size - 58),
> 
> 'Ether(dst="%s")/IPv6(dst="401:101:101:101:101:101:101:101",src="ee80:ee80
> :ee80:ee80:ee80:ee80:ee80:ee80")/Raw(load="X"*%d)' % (dmac, size - 58)]
> -            self.tester.scapy_append('wrpcap("test2.pcap", [%s])' %
> string.join(flows, ','))
> +            self.tester.scapy_append(
> +                'wrpcap("test2.pcap", [%s])' % string.join(flows, ','))
> 
>              self.tester.scapy_execute()
> 
>              tgenInput = []
> -            tgenInput.append((self.tester.get_local_port(P0),
> self.tester.get_local_port(P1), "test1.pcap"))
> -            tgenInput.append((self.tester.get_local_port(P1),
> self.tester.get_local_port(P0), "test2.pcap"))
> +            tgenInput.append(
> +                (self.tester.get_local_port(P0),
> self.tester.get_local_port(P1), "test1.pcap"))
> +            tgenInput.append(
> +                (self.tester.get_local_port(P1),
> self.tester.get_local_port(P0), "test2.pcap"))
> 
>              factor = (size + 1517) / 1518
>              # wireSpd = 2 * 10000.0 / ((20 + size) * 8)
> -            Bps[str(size)], Pps[str(size)] =
> self.tester.traffic_generator_throughput(tgenInput)
> +            Bps[str(size)], Pps[
> +                str(size)] =
> self.tester.traffic_generator_throughput(tgenInput)
>              self.verify(Pps[str(size)] > 0, "No traffic detected")
>              Pps[str(size)] *= 1.0 / factor / 1000000
>              Pct[str(size)] = (1.0 * Bps[str(size)] * 100) / (2 *
> 10000000000)
> @@ -349,7 +393,8 @@ l3fwd_ipv4_route_array[] = {\\\n"
> 
>          self.result_table_create(tblheader)
> 
> -        lcores = [("1S/1C/1T", 2), ("1S/1C/2T", 2), ("1S/2C/1T", 2),
> ("2S/1C/1T", 2)]
> +        lcores = [("1S/1C/1T", 2), ("1S/1C/2T", 2),
> +                  ("1S/2C/1T", 2), ("2S/1C/1T", 2)]
>          index = 1
>          for (lcore, numThr) in lcores:
>              self.benchmark(index, lcore, numThr, sizes)
> @@ -361,8 +406,10 @@ l3fwd_ipv4_route_array[] = {\\\n"
>          """
>          Run after each test case.
>          """
> -        self.tester.send_expect("ifconfig %s mtu 1500" %
> self.tester.get_interface(self.tester.get_local_port(P0)), "#")
> -        self.tester.send_expect("ifconfig %s mtu 1500" %
> self.tester.get_interface(self.tester.get_local_port(P1)), "#")
> +        self.tester.send_expect("ifconfig %s mtu 1500" %
> +
> self.tester.get_interface(self.tester.get_local_port(P0)), "#")
> +        self.tester.send_expect("ifconfig %s mtu 1500" %
> +
> self.tester.get_interface(self.tester.get_local_port(P1)), "#")
> 
>      def tear_down_all(self):
>          """
> --
> 1.9.3

      reply	other threads:[~2017-09-07  2:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-06  9:06 xu,gang
2017-09-07  2:31 ` Liu, Yong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86228AFD5BCD8E4EBFD2B90117B5E81E62E9DAB7@SHSMSX103.ccr.corp.intel.com \
    --to=yong.liu@intel.com \
    --cc=dts@dpdk.org \
    --cc=gangx.xu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).