Re: [dts] [PATCH v3 01/17] framwork/packet: sniff_packet specify running target support

[Phil Yang <Phil.Yang@arm.com>]

Hi Marvin,

> Filename will be return after running load_pcap_f function, please use that
> name.
> It can be multiple sniffer functions work in the same time.

If so, I think it is better to copy sniffer pcap file outside load_tcpdump_sniff_pcap and load_tcpdump_sniff_packets function of tester module.
I was planning to do it that way in patch V2, but it is need to add tester.session.copy_file_from() operation in each test case.

Can I roll back to patch V2?

Thanks,
Phil Yang

> -----Original Message-----
> From: Liu, Yong <yong.liu@intel.com>
> Sent: Wednesday, April 4, 2018 11:28 AM
> To: Phil Yang <Phil.Yang@arm.com>; dts@dpdk.org
> Cc: nd <nd@arm.com>
> Subject: RE: [PATCH v3 01/17] framwork/packet: sniff_packet specify running
> target support
> 
> Phil,
> Some comments are inline.
> 
> Thanks,
> Marvin
> 
> > -----Original Message-----
> > From: Phil Yang [mailto:phil.yang@arm.com]
> > Sent: Monday, April 02, 2018 11:46 AM
> > To: dts@dpdk.org
> > Cc: nd@arm.com; Liu, Yong <yong.liu@intel.com>; phil.yang@arm.com
> > Subject: [PATCH v3 01/17] framwork/packet: sniff_packet specify
> > running target support
> >
> > If tester in crb file was not the machine which running dts, the
> > sniff_packet process will not running on tester.
> >
> > Create a ssh connection to the tester and run tcpdump to make sure
> > sniff_packet process running on the machine we expected.
> >
> > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > ---
> >  framework/packet.py | 34 ++++++++++++++++++++++++++++------
> >  framework/tester.py | 38 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 66 insertions(+), 6 deletions(-)
> >
> > diff --git a/framework/packet.py b/framework/packet.py index
> > 976b82b..484e511 100755
> > --- a/framework/packet.py
> > +++ b/framework/packet.py
> > @@ -812,15 +812,30 @@ def get_filter_cmd(filters=[]):
> >          return ""
> >
> >
> > -def sniff_packets(intf, count=0, timeout=5, filters=[]):
> > +def sniff_packets(intf, count=0, timeout=5, filters=[], target=[]):
> >      """
> >      sniff all packets for certain port in certain seconds
> >      """
> >      param = ""
> >      direct_param = r"(\s+)\[ (\S+) in\|out\|inout \]"
> > -    tcpdump_help = subprocess.check_output("tcpdump -h; echo 0",
> > -                                           stderr=subprocess.STDOUT,
> > -                                           shell=True)
> > +
> > +    # target[] contain the remote machine info for ssh connection
> > +    # target[0]: username
> > +    # target[1]: ip address
> > +    # target[2]: pass word
> > +    if target:
> > +        tcpdump_help_pipe = subprocess.Popen(["ssh",
> > +                            "%s@%s" % (target[0], target[1]),
> > +                            "tcpdump -h"],
> > +                            stderr=subprocess.PIPE,
> > +                            stdout=subprocess.PIPE,
> > +                            shell=False)
> > +        tcpdump_help = "".join(tuple(tcpdump_help_pipe.communicate()))
> > +        tcpdump_help_pipe.wait()
> > +    else:
> > +        tcpdump_help = subprocess.check_output("tcpdump -h; echo 0",
> > +                                    stderr=subprocess.STDOUT,
> > + shell=True)
> > +
> >      for line in tcpdump_help.split('\n'):
> >          m = re.match(direct_param, line)
> >          if m:
> > @@ -850,9 +865,16 @@ def sniff_packets(intf, count=0, timeout=5,
> > filters=[]):
> >      else:
> >          cmd = sniff_cmd % options
> >
> > -    args = shlex.split(cmd)
> > +    if target:
> > +        pipe = subprocess.Popen(["ssh",
> > +                "%s@%s" % (target[0], target[1]),
> > +                cmd],
> > +                stdin=subprocess.PIPE,
> > +                shell=False)
> > +    else:
> > +        args = shlex.split(cmd)
> > +        pipe = subprocess.Popen(args)
> >
> > -    pipe = subprocess.Popen(args)
> >      index = str(time.time())
> >      SNIFF_PIDS[index] = (pipe, intf, timeout)
> >      time.sleep(1)
> > diff --git a/framework/tester.py b/framework/tester.py index
> > a775f68..10761a8 100755
> > --- a/framework/tester.py
> > +++ b/framework/tester.py
> > @@ -70,6 +70,7 @@ class Tester(Crb):
> >          self.bgCmds = []
> >          self.bgItf = ''
> >          self.re_run_time = 0
> > +	self.sniff_intf = ''
> >
> >      def init_ext_gen(self):
> >          """
> > @@ -704,6 +705,43 @@ class Tester(Crb):
> >              self.proc.kill()
> >              self.proc = None
> >
> > +    def tcpdump_sniff_packets(self, intf, count=0, timeout=5, filters=[]):
> > +        """
> > +        Wrapper for packet module sniff_packets
> > +        """
> > +        # load functions in packet module
> > +        module = __import__("packet")
> > +        sniff_f = getattr(module, "sniff_packets")
> > +
> > +        target=[]
> > +        target.append(self.get_username())
> > +        target.append(self.get_ip_address())
> > +        target.append(self.get_password())
> > +	self.sniff_intf = intf
> 
> Indent not align here, and I think there's no need to save sniffer interface in this
> module.
> 
> > +        return sniff_f(intf, count, timeout, filters, target)
> > +
> > +    def load_tcpdump_sniff_pcap(self, index=''):
> > +        """
> > +        Wrapper for packet module load_sniff_pcap
> > +        """
> > +        # load functions in packet module
> > +        module = __import__("packet")
> > +        load_pcap_f = getattr(module, "load_sniff_pcap")
> > +	self.session.copy_file_from("/tmp/sniff_%s.pcap" % self.sniff_intf,
> > "/tmp/")
> > +
> 
> Filename will be return after running load_pcap_f function, please use that
> name.
> It can be multiple sniffer functions work in the same time.
> 
> > +        return load_pcap_f(index)
> > +
> > +    def load_tcpdump_sniff_packets(self, index=''):
> > +        """
> > +        Wrapper for packet module load_sniff_packets
> > +        """
> > +        # load functions in packet module
> > +        module = __import__("packet")
> > +        load_f = getattr(module, "load_sniff_packets")
> > +	self.session.copy_file_from("/tmp/sniff_%s.pcap" % self.sniff_intf,
> > "/tmp/")
> > +
> > +        return load_f(index)
> > +
> >      def kill_all(self, killall=False):
> >          """
> >          Kill all scapy process or DPDK application on tester.
> > --
> > 2.7.4