[ovs-dev,v3,ovn,2/3] ovn-detrace: Fix line parsing.

The script was never parsing the first cookie in the input. Also, add a
check to make sure that the cookie refers to a Logical_Flow before
trying to print the record.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 utilities/ovn-detrace.in |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Hi Dumitru,

Thanks for improving the tool.

On Tue, Nov 12, 2019 at 8:47 AM Dumitru Ceara <dceara@redhat.com> wrote:
>
> The script was never parsing the first cookie in the input. Also, add a
> check to make sure that the cookie refers to a Logical_Flow before
> trying to print the record.
>
> Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> ---
>  utilities/ovn-detrace.in |   24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
> index 9471e37..34b6b0e 100755
> --- a/utilities/ovn-detrace.in
> +++ b/utilities/ovn-detrace.in
> @@ -188,22 +188,26 @@ def main():
>      cookie = None
>      while True:
>          line = sys.stdin.readline()
> +
> +        if line == '':
> +            break
> +
> +        line = line.strip()
> +
>          if cookie:
>              # print lflow info when the current flow block ends
> -            if regex_table_id.match(line) or line.strip() == '':
> +            if regex_table_id.match(line):
>                  lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie)
> -                print_lflow(lflow, "  * ")
> -                print_lflow_nb_hint(lflow, "    * ", ovsdb_ovnnb)
> -                cookie = None
> +                if lflow:
> +                    print_lflow(lflow, "  * ")
> +                    print_lflow_nb_hint(lflow, "    * ", ovsdb_ovnnb)
> +                    cookie = None
>
> -        print line.strip()
> -        if line == "":
> -            break
> +        print line
>
>          m = regex_cookie.match(line)
> -        if not m:
> -            continue
> -        cookie = m.group(1)
> +        if m:
> +            cookie = m.group(1)
>
>
>  if __name__ == "__main__":
>

Maybe I missed something here, but the original logic seems to be correct
to me. It always parses the current line - if there is a cookie, it is
parsed. And then it prints the previous logical flow information using the
last cookie that was parsed when a *new* flow or line break is encountered.
And finally, it ensures the last logical flow information is printed
because the "break" is AFTER the "if cookie" block.
Now with the change, it seems the last logical flow (related to the last
cookie) may not be printed, if it happens to be the last line. (although it
is not a problem if the last line doesn't has cookie)

Thanks,
Han

On Tue, Nov 12, 2019 at 11:16 PM Han Zhou <hzhou@ovn.org> wrote:
>
>
> Hi Dumitru,
>
> Thanks for improving the tool.

Hi Han,

Thanks for the review.

>
> On Tue, Nov 12, 2019 at 8:47 AM Dumitru Ceara <dceara@redhat.com> wrote:
> >
> > The script was never parsing the first cookie in the input. Also, add a
> > check to make sure that the cookie refers to a Logical_Flow before
> > trying to print the record.
> >
> > Signed-off-by: Dumitru Ceara <dceara@redhat.com>
> > ---
> >  utilities/ovn-detrace.in |   24 ++++++++++++++----------
> >  1 file changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in
> > index 9471e37..34b6b0e 100755
> > --- a/utilities/ovn-detrace.in
> > +++ b/utilities/ovn-detrace.in
> > @@ -188,22 +188,26 @@ def main():
> >      cookie = None
> >      while True:
> >          line = sys.stdin.readline()
> > +
> > +        if line == '':
> > +            break
> > +
> > +        line = line.strip()
> > +
> >          if cookie:
> >              # print lflow info when the current flow block ends
> > -            if regex_table_id.match(line) or line.strip() == '':
> > +            if regex_table_id.match(line):
> >                  lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie)
> > -                print_lflow(lflow, "  * ")
> > -                print_lflow_nb_hint(lflow, "    * ", ovsdb_ovnnb)
> > -                cookie = None
> > +                if lflow:
> > +                    print_lflow(lflow, "  * ")
> > +                    print_lflow_nb_hint(lflow, "    * ", ovsdb_ovnnb)
> > +                    cookie = None
> >
> > -        print line.strip()
> > -        if line == "":
> > -            break
> > +        print line
> >
> >          m = regex_cookie.match(line)
> > -        if not m:
> > -            continue
> > -        cookie = m.group(1)
> > +        if m:
> > +            cookie = m.group(1)
> >
> >
> >  if __name__ == "__main__":
> >
>
> Maybe I missed something here, but the original logic seems to be correct to me. It always parses the current line - if there is a cookie, it is parsed. And then it prints the previous logical flow information using the last cookie that was parsed when a *new* flow or line break is encountered. And finally, it ensures the last logical flow information is printed because the "break" is AFTER the "if cookie" block.

With the following (simplified) input the original code doesn't work:
$ cat /tmp/test_input
 0. in_port=1, priority 100, cookie 0x6b55ca8d
 8. reg14=0x2,metadata=0x3, priority 50, cookie 0xf0b23940
 9. metadata=0x3, priority 0, cookie 0x892dcbdd
10. metadata=0x3, priority 0, cookie 0xf35a08d5
11. ip,metadata=0x3, priority 100, cookie 0x627415f1

In the first three lines we don't match regex_table_id.match(line)
because of the leading whitespace.

> Now with the change, it seems the last logical flow (related to the last cookie) may not be printed, if it happens to be the last line. (although it is not a problem if the last line doesn't has cookie)

My bad, I'll send a v4 taking care of this case too.

Thanks,
Dumitru

>
> Thanks,
> Han