mbox series

[ovs-dev,v4,00/12] Add flow visualization utility.

Message ID 20240507143044.3056232-1-amorenoz@redhat.com
Headers show
Series Add flow visualization utility. | expand

Message

Adrián Moreno May 7, 2024, 2:30 p.m. UTC
The goal of this utility is to read both datapath and Openflow flows
(using the flow library already available) and print them in different
formats and styles to make it easier to understand them and troubleshoot
issues.

The formats are quite opinionated and so are the colors chosen so I'm
eager to hear what is the impression caused to the community.

Here is a summary of the formats and features supported:

- Openflow
   - Console: Prints flows back to the console allowing filtering and
     extensive formatting.
   - Cookie: Arranges flows based on cookie (instead of table) to ease
     visualization of OVN-generated flows.
   - HTML: Prints an HTML file that shows the flows arranged by tables.
     resubmit actions have a hyperlinke to the target table to ease
     navegation.
   - Logic: Many times OVN generates many "logically-equivalent" flows.
     These are flows that have the same structure: match on the same
     values and have the same actions. The only thing that changes is
     the actual value they match against and maybe the arguments of the
     actions. This format collapses these flows so you can visualize the
     logical pipeline easier.
   - JSON: JSON format.

More OpenFlow features:
   - OVN metadata: Both the "cookie" and the "logic" format allow to
     connect to a running OVN NB/SB databases and enrich the flows with
     OVN context based on the flow's cookie.

- Datapath:
   - Console: Prints flows back to the console allowing filtering and
     extensive formatting.
   - Tree: Datapath flows use recirculation to further execute
     additional actions. This format builds a tree of flows following
     the recirculation identifiers and prints it in the console.
   - HTML: Prints the datapath flow tree in HTML. It includes some
     minimal JS to support expanding and collapsing of entire branches.
   - Graph: Following the "tree" format, this one prints the tree in
     graphviz format. 
   - JSON: JSON format.

Additional datapath features:
   - Many datapath formats are based on the tree flow hierarchy. An
     interesting feature of this structure is that filtering is done at
     the branch level. This means that when a flow satisfies the filter,
     the entire sub-tree leading to that flow is shown.

Additional common features:
   - Styling: Styles for both console and HTML formats can be defined
     using a configuration file.
   - Heat maps: Some formats support heat-maps. A color pallete ranging
     from blue (cold) to red (hot) is used to print the number of
     packets and bytes of the flows. That way, the flows that are
     handling more traffic are much easier to spot

--
V3 -> V4:
 - Add manpage to rpm package
 - Fix Eelco's comments
V2 -> V3:
 - Fix grammar thanks to Eelco's review
V1 -> V2:
 - Fix typos and nits on documentation
 - Split documentation in two: ovs-flowviz.8 man page and a topic
   article with more detailed examples.
RFC -> V1:
 - Addressed Eelco's comments
 - Added a documentation page
 - Added support for dark style HTML pages
 - Patch 3. Small fix in the way a style is looked up. Use the key in
   the KV instead of the metadata string. This helps with "free" values
   such as "output".

Adrian Moreno (12):
  python: ovs: Add flowviz scheleton.
  python: ovs: flowviz: Add file processing infra.
  python: ovs: flowviz: Add console formatting.
  python: ovs: flowviz: Add default config file.
  python: ovs: flowviz: Add html formatting.
  python: ovs: flowviz: Add datapath tree format.
  python: ovs: flowviz: Add OpenFlow logical view.
  python: ovs: flowviz: Add Openflow cookie format.
  python: ovs: flowviz: Add datapath html format.
  python: ovs: flowviz: Add datapath graph format.
  python: ovs: flowviz: Support html dark style.
  documentation: Document ovs-flowviz.

 Documentation/automake.mk                   |   4 +-
 Documentation/conf.py                       |   2 +
 Documentation/ref/index.rst                 |   1 +
 Documentation/ref/ovs-flowviz.8.rst         | 531 ++++++++++++++++++++
 Documentation/topics/flow-visualization.rst | 271 ++++++++++
 Documentation/topics/index.rst              |   1 +
 python/automake.mk                          |  32 +-
 python/ovs/flowviz/__init__.py              |   2 +
 python/ovs/flowviz/console.py               | 196 ++++++++
 python/ovs/flowviz/format.py                | 371 ++++++++++++++
 python/ovs/flowviz/html_format.py           | 138 +++++
 python/ovs/flowviz/main.py                  | 196 ++++++++
 python/ovs/flowviz/odp/__init__.py          |   0
 python/ovs/flowviz/odp/cli.py               | 116 +++++
 python/ovs/flowviz/odp/graph.py             | 418 +++++++++++++++
 python/ovs/flowviz/odp/html.py              | 279 ++++++++++
 python/ovs/flowviz/odp/tree.py              | 303 +++++++++++
 python/ovs/flowviz/ofp/__init__.py          |   0
 python/ovs/flowviz/ofp/cli.py               | 246 +++++++++
 python/ovs/flowviz/ofp/html.py              |  98 ++++
 python/ovs/flowviz/ofp/logic.py             | 364 ++++++++++++++
 python/ovs/flowviz/ovs-flowviz              |  20 +
 python/ovs/flowviz/ovs-flowviz.conf         | 128 +++++
 python/ovs/flowviz/process.py               | 263 ++++++++++
 python/setup.py                             |  14 +-
 rhel/openvswitch-fedora.spec.in             |   1 +
 rhel/openvswitch.spec.in                    |   1 +
 27 files changed, 3986 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/ref/ovs-flowviz.8.rst
 create mode 100644 Documentation/topics/flow-visualization.rst
 create mode 100644 python/ovs/flowviz/__init__.py
 create mode 100644 python/ovs/flowviz/console.py
 create mode 100644 python/ovs/flowviz/format.py
 create mode 100644 python/ovs/flowviz/html_format.py
 create mode 100644 python/ovs/flowviz/main.py
 create mode 100644 python/ovs/flowviz/odp/__init__.py
 create mode 100644 python/ovs/flowviz/odp/cli.py
 create mode 100644 python/ovs/flowviz/odp/graph.py
 create mode 100644 python/ovs/flowviz/odp/html.py
 create mode 100644 python/ovs/flowviz/odp/tree.py
 create mode 100644 python/ovs/flowviz/ofp/__init__.py
 create mode 100644 python/ovs/flowviz/ofp/cli.py
 create mode 100644 python/ovs/flowviz/ofp/html.py
 create mode 100644 python/ovs/flowviz/ofp/logic.py
 create mode 100755 python/ovs/flowviz/ovs-flowviz
 create mode 100644 python/ovs/flowviz/ovs-flowviz.conf
 create mode 100644 python/ovs/flowviz/process.py

Comments

Ilya Maximets May 21, 2024, 11:51 a.m. UTC | #1
On 5/7/24 16:30, Adrian Moreno wrote:
> The goal of this utility is to read both datapath and Openflow flows
> (using the flow library already available) and print them in different
> formats and styles to make it easier to understand them and troubleshoot
> issues.
> 
> The formats are quite opinionated and so are the colors chosen so I'm
> eager to hear what is the impression caused to the community.
> 
> Here is a summary of the formats and features supported:
> 
> - Openflow
>    - Console: Prints flows back to the console allowing filtering and
>      extensive formatting.
>    - Cookie: Arranges flows based on cookie (instead of table) to ease
>      visualization of OVN-generated flows.
>    - HTML: Prints an HTML file that shows the flows arranged by tables.
>      resubmit actions have a hyperlinke to the target table to ease
>      navegation.
>    - Logic: Many times OVN generates many "logically-equivalent" flows.
>      These are flows that have the same structure: match on the same
>      values and have the same actions. The only thing that changes is
>      the actual value they match against and maybe the arguments of the
>      actions. This format collapses these flows so you can visualize the
>      logical pipeline easier.
>    - JSON: JSON format.
> 
> More OpenFlow features:
>    - OVN metadata: Both the "cookie" and the "logic" format allow to
>      connect to a running OVN NB/SB databases and enrich the flows with
>      OVN context based on the flow's cookie.
> 
> - Datapath:
>    - Console: Prints flows back to the console allowing filtering and
>      extensive formatting.
>    - Tree: Datapath flows use recirculation to further execute
>      additional actions. This format builds a tree of flows following
>      the recirculation identifiers and prints it in the console.
>    - HTML: Prints the datapath flow tree in HTML. It includes some
>      minimal JS to support expanding and collapsing of entire branches.
>    - Graph: Following the "tree" format, this one prints the tree in
>      graphviz format. 
>    - JSON: JSON format.
> 
> Additional datapath features:
>    - Many datapath formats are based on the tree flow hierarchy. An
>      interesting feature of this structure is that filtering is done at
>      the branch level. This means that when a flow satisfies the filter,
>      the entire sub-tree leading to that flow is shown.
> 
> Additional common features:
>    - Styling: Styles for both console and HTML formats can be defined
>      using a configuration file.
>    - Heat maps: Some formats support heat-maps. A color pallete ranging
>      from blue (cold) to red (hot) is used to print the number of
>      packets and bytes of the flows. That way, the flows that are
>      handling more traffic are much easier to spot
> 
> --
> V3 -> V4:
>  - Add manpage to rpm package
>  - Fix Eelco's comments
> V2 -> V3:
>  - Fix grammar thanks to Eelco's review
> V1 -> V2:
>  - Fix typos and nits on documentation
>  - Split documentation in two: ovs-flowviz.8 man page and a topic
>    article with more detailed examples.
> RFC -> V1:
>  - Addressed Eelco's comments
>  - Added a documentation page
>  - Added support for dark style HTML pages
>  - Patch 3. Small fix in the way a style is looked up. Use the key in
>    the KV instead of the metadata string. This helps with "free" values
>    such as "output".
> 
> Adrian Moreno (12):
>   python: ovs: Add flowviz scheleton.
>   python: ovs: flowviz: Add file processing infra.
>   python: ovs: flowviz: Add console formatting.
>   python: ovs: flowviz: Add default config file.
>   python: ovs: flowviz: Add html formatting.
>   python: ovs: flowviz: Add datapath tree format.
>   python: ovs: flowviz: Add OpenFlow logical view.
>   python: ovs: flowviz: Add Openflow cookie format.
>   python: ovs: flowviz: Add datapath html format.
>   python: ovs: flowviz: Add datapath graph format.
>   python: ovs: flowviz: Support html dark style.
>   documentation: Document ovs-flowviz.
> 
>  Documentation/automake.mk                   |   4 +-
>  Documentation/conf.py                       |   2 +
>  Documentation/ref/index.rst                 |   1 +
>  Documentation/ref/ovs-flowviz.8.rst         | 531 ++++++++++++++++++++
>  Documentation/topics/flow-visualization.rst | 271 ++++++++++
>  Documentation/topics/index.rst              |   1 +
>  python/automake.mk                          |  32 +-
>  python/ovs/flowviz/__init__.py              |   2 +
>  python/ovs/flowviz/console.py               | 196 ++++++++
>  python/ovs/flowviz/format.py                | 371 ++++++++++++++
>  python/ovs/flowviz/html_format.py           | 138 +++++
>  python/ovs/flowviz/main.py                  | 196 ++++++++
>  python/ovs/flowviz/odp/__init__.py          |   0
>  python/ovs/flowviz/odp/cli.py               | 116 +++++
>  python/ovs/flowviz/odp/graph.py             | 418 +++++++++++++++
>  python/ovs/flowviz/odp/html.py              | 279 ++++++++++
>  python/ovs/flowviz/odp/tree.py              | 303 +++++++++++
>  python/ovs/flowviz/ofp/__init__.py          |   0
>  python/ovs/flowviz/ofp/cli.py               | 246 +++++++++
>  python/ovs/flowviz/ofp/html.py              |  98 ++++
>  python/ovs/flowviz/ofp/logic.py             | 364 ++++++++++++++
>  python/ovs/flowviz/ovs-flowviz              |  20 +
>  python/ovs/flowviz/ovs-flowviz.conf         | 128 +++++
>  python/ovs/flowviz/process.py               | 263 ++++++++++
>  python/setup.py                             |  14 +-
>  rhel/openvswitch-fedora.spec.in             |   1 +
>  rhel/openvswitch.spec.in                    |   1 +
>  27 files changed, 3986 insertions(+), 10 deletions(-)
>  create mode 100644 Documentation/ref/ovs-flowviz.8.rst
>  create mode 100644 Documentation/topics/flow-visualization.rst
>  create mode 100644 python/ovs/flowviz/__init__.py
>  create mode 100644 python/ovs/flowviz/console.py
>  create mode 100644 python/ovs/flowviz/format.py
>  create mode 100644 python/ovs/flowviz/html_format.py
>  create mode 100644 python/ovs/flowviz/main.py
>  create mode 100644 python/ovs/flowviz/odp/__init__.py
>  create mode 100644 python/ovs/flowviz/odp/cli.py
>  create mode 100644 python/ovs/flowviz/odp/graph.py
>  create mode 100644 python/ovs/flowviz/odp/html.py
>  create mode 100644 python/ovs/flowviz/odp/tree.py
>  create mode 100644 python/ovs/flowviz/ofp/__init__.py
>  create mode 100644 python/ovs/flowviz/ofp/cli.py
>  create mode 100644 python/ovs/flowviz/ofp/html.py
>  create mode 100644 python/ovs/flowviz/ofp/logic.py
>  create mode 100755 python/ovs/flowviz/ovs-flowviz
>  create mode 100644 python/ovs/flowviz/ovs-flowviz.conf
>  create mode 100644 python/ovs/flowviz/process.py
> 

Hi, Adrian.  Thanks for v4!

Not a full review, but I was trying to use this version of flowviz today
on a real flow dump from an ovn-kubernetes cluster and it crashes unable
to parse check_pkt_len datapath actions:

# ovs-flowviz -i snippet1.txt datapath tree                                     

Traceback (most recent call last):                                                                                                                                           
  File "/python/ovs/flow/kv.py", line 294, in parse                                          
    key, val = self._decoders.decode(keyword, value_str)                                                                                                                     
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                     
  File "/python/ovs/flow/kv.py", line 150, in decode                                         
    raise ParseError(                                                                                                                                                        
ovs.flow.kv.ParseError: Cannot parse key check_pkt_len: No decoder found                                                                                                     
                                                                                                                                                                             
The above exception was the direct cause of the following exception:                                                                                                         
                                                                                                                                                                             
Traceback (most recent call last):                                                                                                                                           
  File "/python/ovs/flow/kv.py", line 294, in parse                                          
    key, val = self._decoders.decode(keyword, value_str)                                                                                                                     
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                     
  File "/python/ovs/flow/kv.py", line 137, in decode                                         
    result = decoder(value_str)                                                                                                                                              
             ^^^^^^^^^^^^^^^^^^                                                                                                                                              
  File "/python/ovs/flow/kv.py", line 347, in decode_nested_kv_list                          
    parser.parse()                                                                                                                                                           
  File "/python/ovs/flow/kv.py", line 296, in parse                                          
    raise ParseError(                                                                                                                                                        
ovs.flow.kv.ParseError: Error parsing key-value (check_pkt_len, size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,c
ontinuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204b07e)))                                            
                                                                                                                                                                             
The above exception was the direct cause of the following exception:                                                                                                         
                                                                                                                    
The above exception was the direct cause of the following exception:                                                                                                         
                                                                                                                                                                             
Traceback (most recent call last):                                                                                                                                           
  File "/python/ovs/flow/kv.py", line 294, in parse                                          
    key, val = self._decoders.decode(keyword, value_str)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/python/ovs/flow/kv.py", line 137, in decode
    result = decoder(value_str)
             ^^^^^^^^^^^^^^^^^^
  File "/python/ovs/flow/kv.py", line 330, in decode_nested_kv
    parser.parse()
  File "/python/ovs/flow/kv.py", line 296, in parse
    raise ParseError(
ovs.flow.kv.ParseError: Error parsing key-value (le, set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,
actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(z
one=12,nat),recirc(0x204b07e))))

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/venv/bin/ovs-flowviz", line 20, in <module>
    main.main()
  File "/python/ovs/flowviz/main.py", line 196, in main
    maincli()
  File "/venv/lib64/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib64/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
File "/venv/lib64/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib64/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib64/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib64/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/lib64/python3.12/site-packages/click/decorators.py", line 45, in new_func
    return f(get_current_context().obj, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/python/ovs/flowviz/odp/cli.py", line 85, in tree
    processor.process()
  File "/python/ovs/flowviz/odp/tree.py", line 233, in process
    super().process(False)
  File "/python/ovs/flowviz/process.py", line 113, in process
    flow = self.create_flow(line, idx)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/python/ovs/flowviz/process.py", line 158, in create_flow
    return ODPFlow(line, idx)
           ^^^^^^^^^^^^^^^^^^
  File "/python/ovs/flow/odp.py", line 153, in __init__
    aparser.parse()
  File "/python/ovs/flow/kv.py", line 296, in parse
    raise ParseError(
ovs.flow.kv.ParseError: Error parsing key-value (check_pkt_len, size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204b07e)))))


Not the exact same line, but a similar crash happens while parsing:

recirc_id(0x204ae73),in_port(20),ct_state(+new-est-rel-rpl-inv+trk),ct_mark(0/0xffff000f),eth(src=1a:1b:1c:1d:18:18,dst=2a:2b:2c:2d:2e:2f),eth_type(0x0800),ipv4(src=192.168.10.1/255.255.255.254,dst=10.64.0.0/255.192.0.0,proto=6,ttl=64,frag=no), packets:7, bytes:518, used:9.077s, flags:S, actions:ct(commit,zone=269,mark=0/0x1,nat(src)),set(eth(src=3a:3b:3c:3d:3e:3f,dst=4a:4b:4c:4d:4e:4f)),set(ipv4(ttl=63)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861236,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204ae75)))))

My guess is that it has some trouble with nested check_pkt_len.

Best regards, Ilya Maximets.
Adrián Moreno May 21, 2024, 3:49 p.m. UTC | #2
On Tue, May 21, 2024 at 01:51:33PM GMT, Ilya Maximets wrote:
> On 5/7/24 16:30, Adrian Moreno wrote:
> > The goal of this utility is to read both datapath and Openflow flows
> > (using the flow library already available) and print them in different
> > formats and styles to make it easier to understand them and troubleshoot
> > issues.
> >
> > The formats are quite opinionated and so are the colors chosen so I'm
> > eager to hear what is the impression caused to the community.
> >
> > Here is a summary of the formats and features supported:
> >
> > - Openflow
> >    - Console: Prints flows back to the console allowing filtering and
> >      extensive formatting.
> >    - Cookie: Arranges flows based on cookie (instead of table) to ease
> >      visualization of OVN-generated flows.
> >    - HTML: Prints an HTML file that shows the flows arranged by tables.
> >      resubmit actions have a hyperlinke to the target table to ease
> >      navegation.
> >    - Logic: Many times OVN generates many "logically-equivalent" flows.
> >      These are flows that have the same structure: match on the same
> >      values and have the same actions. The only thing that changes is
> >      the actual value they match against and maybe the arguments of the
> >      actions. This format collapses these flows so you can visualize the
> >      logical pipeline easier.
> >    - JSON: JSON format.
> >
> > More OpenFlow features:
> >    - OVN metadata: Both the "cookie" and the "logic" format allow to
> >      connect to a running OVN NB/SB databases and enrich the flows with
> >      OVN context based on the flow's cookie.
> >
> > - Datapath:
> >    - Console: Prints flows back to the console allowing filtering and
> >      extensive formatting.
> >    - Tree: Datapath flows use recirculation to further execute
> >      additional actions. This format builds a tree of flows following
> >      the recirculation identifiers and prints it in the console.
> >    - HTML: Prints the datapath flow tree in HTML. It includes some
> >      minimal JS to support expanding and collapsing of entire branches.
> >    - Graph: Following the "tree" format, this one prints the tree in
> >      graphviz format.
> >    - JSON: JSON format.
> >
> > Additional datapath features:
> >    - Many datapath formats are based on the tree flow hierarchy. An
> >      interesting feature of this structure is that filtering is done at
> >      the branch level. This means that when a flow satisfies the filter,
> >      the entire sub-tree leading to that flow is shown.
> >
> > Additional common features:
> >    - Styling: Styles for both console and HTML formats can be defined
> >      using a configuration file.
> >    - Heat maps: Some formats support heat-maps. A color pallete ranging
> >      from blue (cold) to red (hot) is used to print the number of
> >      packets and bytes of the flows. That way, the flows that are
> >      handling more traffic are much easier to spot
> >
> > --
> > V3 -> V4:
> >  - Add manpage to rpm package
> >  - Fix Eelco's comments
> > V2 -> V3:
> >  - Fix grammar thanks to Eelco's review
> > V1 -> V2:
> >  - Fix typos and nits on documentation
> >  - Split documentation in two: ovs-flowviz.8 man page and a topic
> >    article with more detailed examples.
> > RFC -> V1:
> >  - Addressed Eelco's comments
> >  - Added a documentation page
> >  - Added support for dark style HTML pages
> >  - Patch 3. Small fix in the way a style is looked up. Use the key in
> >    the KV instead of the metadata string. This helps with "free" values
> >    such as "output".
> >
> > Adrian Moreno (12):
> >   python: ovs: Add flowviz scheleton.
> >   python: ovs: flowviz: Add file processing infra.
> >   python: ovs: flowviz: Add console formatting.
> >   python: ovs: flowviz: Add default config file.
> >   python: ovs: flowviz: Add html formatting.
> >   python: ovs: flowviz: Add datapath tree format.
> >   python: ovs: flowviz: Add OpenFlow logical view.
> >   python: ovs: flowviz: Add Openflow cookie format.
> >   python: ovs: flowviz: Add datapath html format.
> >   python: ovs: flowviz: Add datapath graph format.
> >   python: ovs: flowviz: Support html dark style.
> >   documentation: Document ovs-flowviz.
> >
> >  Documentation/automake.mk                   |   4 +-
> >  Documentation/conf.py                       |   2 +
> >  Documentation/ref/index.rst                 |   1 +
> >  Documentation/ref/ovs-flowviz.8.rst         | 531 ++++++++++++++++++++
> >  Documentation/topics/flow-visualization.rst | 271 ++++++++++
> >  Documentation/topics/index.rst              |   1 +
> >  python/automake.mk                          |  32 +-
> >  python/ovs/flowviz/__init__.py              |   2 +
> >  python/ovs/flowviz/console.py               | 196 ++++++++
> >  python/ovs/flowviz/format.py                | 371 ++++++++++++++
> >  python/ovs/flowviz/html_format.py           | 138 +++++
> >  python/ovs/flowviz/main.py                  | 196 ++++++++
> >  python/ovs/flowviz/odp/__init__.py          |   0
> >  python/ovs/flowviz/odp/cli.py               | 116 +++++
> >  python/ovs/flowviz/odp/graph.py             | 418 +++++++++++++++
> >  python/ovs/flowviz/odp/html.py              | 279 ++++++++++
> >  python/ovs/flowviz/odp/tree.py              | 303 +++++++++++
> >  python/ovs/flowviz/ofp/__init__.py          |   0
> >  python/ovs/flowviz/ofp/cli.py               | 246 +++++++++
> >  python/ovs/flowviz/ofp/html.py              |  98 ++++
> >  python/ovs/flowviz/ofp/logic.py             | 364 ++++++++++++++
> >  python/ovs/flowviz/ovs-flowviz              |  20 +
> >  python/ovs/flowviz/ovs-flowviz.conf         | 128 +++++
> >  python/ovs/flowviz/process.py               | 263 ++++++++++
> >  python/setup.py                             |  14 +-
> >  rhel/openvswitch-fedora.spec.in             |   1 +
> >  rhel/openvswitch.spec.in                    |   1 +
> >  27 files changed, 3986 insertions(+), 10 deletions(-)
> >  create mode 100644 Documentation/ref/ovs-flowviz.8.rst
> >  create mode 100644 Documentation/topics/flow-visualization.rst
> >  create mode 100644 python/ovs/flowviz/__init__.py
> >  create mode 100644 python/ovs/flowviz/console.py
> >  create mode 100644 python/ovs/flowviz/format.py
> >  create mode 100644 python/ovs/flowviz/html_format.py
> >  create mode 100644 python/ovs/flowviz/main.py
> >  create mode 100644 python/ovs/flowviz/odp/__init__.py
> >  create mode 100644 python/ovs/flowviz/odp/cli.py
> >  create mode 100644 python/ovs/flowviz/odp/graph.py
> >  create mode 100644 python/ovs/flowviz/odp/html.py
> >  create mode 100644 python/ovs/flowviz/odp/tree.py
> >  create mode 100644 python/ovs/flowviz/ofp/__init__.py
> >  create mode 100644 python/ovs/flowviz/ofp/cli.py
> >  create mode 100644 python/ovs/flowviz/ofp/html.py
> >  create mode 100644 python/ovs/flowviz/ofp/logic.py
> >  create mode 100755 python/ovs/flowviz/ovs-flowviz
> >  create mode 100644 python/ovs/flowviz/ovs-flowviz.conf
> >  create mode 100644 python/ovs/flowviz/process.py
> >
>
> Hi, Adrian.  Thanks for v4!
>
> Not a full review, but I was trying to use this version of flowviz today
> on a real flow dump from an ovn-kubernetes cluster and it crashes unable
> to parse check_pkt_len datapath actions:
>
> # ovs-flowviz -i snippet1.txt datapath tree
>
> Traceback (most recent call last):
>   File "/python/ovs/flow/kv.py", line 294, in parse
>     key, val = self._decoders.decode(keyword, value_str)
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/python/ovs/flow/kv.py", line 150, in decode
>     raise ParseError(
> ovs.flow.kv.ParseError: Cannot parse key check_pkt_len: No decoder found
>
> The above exception was the direct cause of the following exception:
>
> Traceback (most recent call last):
>   File "/python/ovs/flow/kv.py", line 294, in parse
>     key, val = self._decoders.decode(keyword, value_str)
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/python/ovs/flow/kv.py", line 137, in decode
>     result = decoder(value_str)
>              ^^^^^^^^^^^^^^^^^^
>   File "/python/ovs/flow/kv.py", line 347, in decode_nested_kv_list
>     parser.parse()
>   File "/python/ovs/flow/kv.py", line 296, in parse
>     raise ParseError(
> ovs.flow.kv.ParseError: Error parsing key-value (check_pkt_len, size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,c
> ontinuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204b07e)))
>
> The above exception was the direct cause of the following exception:
>
> The above exception was the direct cause of the following exception:
>
> Traceback (most recent call last):
>   File "/python/ovs/flow/kv.py", line 294, in parse
>     key, val = self._decoders.decode(keyword, value_str)
>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/python/ovs/flow/kv.py", line 137, in decode
>     result = decoder(value_str)
>              ^^^^^^^^^^^^^^^^^^
>   File "/python/ovs/flow/kv.py", line 330, in decode_nested_kv
>     parser.parse()
>   File "/python/ovs/flow/kv.py", line 296, in parse
>     raise ParseError(
> ovs.flow.kv.ParseError: Error parsing key-value (le, set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,
> actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(z
> one=12,nat),recirc(0x204b07e))))
>
> The above exception was the direct cause of the following exception:
>
> Traceback (most recent call last):
>   File "/venv/bin/ovs-flowviz", line 20, in <module>
>     main.main()
>   File "/python/ovs/flowviz/main.py", line 196, in main
>     maincli()
>   File "/venv/lib64/python3.12/site-packages/click/core.py", line 1157, in __call__
>     return self.main(*args, **kwargs)
>            ^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/venv/lib64/python3.12/site-packages/click/core.py", line 1078, in main
>     rv = self.invoke(ctx)
>          ^^^^^^^^^^^^^^^^
> File "/venv/lib64/python3.12/site-packages/click/core.py", line 1688, in invoke
>     return _process_result(sub_ctx.command.invoke(sub_ctx))
>                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/venv/lib64/python3.12/site-packages/click/core.py", line 1688, in invoke
>     return _process_result(sub_ctx.command.invoke(sub_ctx))
>                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/venv/lib64/python3.12/site-packages/click/core.py", line 1434, in invoke
>     return ctx.invoke(self.callback, **ctx.params)
>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/venv/lib64/python3.12/site-packages/click/core.py", line 783, in invoke
>     return __callback(*args, **kwargs)
>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/venv/lib64/python3.12/site-packages/click/decorators.py", line 45, in new_func
>     return f(get_current_context().obj, *args, **kwargs)
>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/python/ovs/flowviz/odp/cli.py", line 85, in tree
>     processor.process()
>   File "/python/ovs/flowviz/odp/tree.py", line 233, in process
>     super().process(False)
>   File "/python/ovs/flowviz/process.py", line 113, in process
>     flow = self.create_flow(line, idx)
>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File "/python/ovs/flowviz/process.py", line 158, in create_flow
>     return ODPFlow(line, idx)
>            ^^^^^^^^^^^^^^^^^^
>   File "/python/ovs/flow/odp.py", line 153, in __init__
>     aparser.parse()
>   File "/python/ovs/flow/kv.py", line 296, in parse
>     raise ParseError(
> ovs.flow.kv.ParseError: Error parsing key-value (check_pkt_len, size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204b07e)))))
>
>
> Not the exact same line, but a similar crash happens while parsing:
>
> recirc_id(0x204ae73),in_port(20),ct_state(+new-est-rel-rpl-inv+trk),ct_mark(0/0xffff000f),eth(src=1a:1b:1c:1d:18:18,dst=2a:2b:2c:2d:2e:2f),eth_type(0x0800),ipv4(src=192.168.10.1/255.255.255.254,dst=10.64.0.0/255.192.0.0,proto=6,ttl=64,frag=no), packets:7, bytes:518, used:9.077s, flags:S, actions:ct(commit,zone=269,mark=0/0x1,nat(src)),set(eth(src=3a:3b:3c:3d:3e:3f,dst=4a:4b:4c:4d:4e:4f)),set(ipv4(ttl=63)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861236,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204ae75)))))
>
> My guess is that it has some trouble with nested check_pkt_len.
>

Thanks. I'll take a look.

> Best regards, Ilya Maximets.
>
Adrián Moreno May 21, 2024, 4:13 p.m. UTC | #3
On Tue, May 21, 2024 at 03:49:03PM GMT, amorenoz@redhat.com wrote:
> On Tue, May 21, 2024 at 01:51:33PM GMT, Ilya Maximets wrote:
> > On 5/7/24 16:30, Adrian Moreno wrote:
> > > The goal of this utility is to read both datapath and Openflow flows
> > > (using the flow library already available) and print them in different
> > > formats and styles to make it easier to understand them and troubleshoot
> > > issues.
> > >
> > > The formats are quite opinionated and so are the colors chosen so I'm
> > > eager to hear what is the impression caused to the community.
> > >
> > > Here is a summary of the formats and features supported:
> > >
> > > - Openflow
> > >    - Console: Prints flows back to the console allowing filtering and
> > >      extensive formatting.
> > >    - Cookie: Arranges flows based on cookie (instead of table) to ease
> > >      visualization of OVN-generated flows.
> > >    - HTML: Prints an HTML file that shows the flows arranged by tables.
> > >      resubmit actions have a hyperlinke to the target table to ease
> > >      navegation.
> > >    - Logic: Many times OVN generates many "logically-equivalent" flows.
> > >      These are flows that have the same structure: match on the same
> > >      values and have the same actions. The only thing that changes is
> > >      the actual value they match against and maybe the arguments of the
> > >      actions. This format collapses these flows so you can visualize the
> > >      logical pipeline easier.
> > >    - JSON: JSON format.
> > >
> > > More OpenFlow features:
> > >    - OVN metadata: Both the "cookie" and the "logic" format allow to
> > >      connect to a running OVN NB/SB databases and enrich the flows with
> > >      OVN context based on the flow's cookie.
> > >
> > > - Datapath:
> > >    - Console: Prints flows back to the console allowing filtering and
> > >      extensive formatting.
> > >    - Tree: Datapath flows use recirculation to further execute
> > >      additional actions. This format builds a tree of flows following
> > >      the recirculation identifiers and prints it in the console.
> > >    - HTML: Prints the datapath flow tree in HTML. It includes some
> > >      minimal JS to support expanding and collapsing of entire branches.
> > >    - Graph: Following the "tree" format, this one prints the tree in
> > >      graphviz format.
> > >    - JSON: JSON format.
> > >
> > > Additional datapath features:
> > >    - Many datapath formats are based on the tree flow hierarchy. An
> > >      interesting feature of this structure is that filtering is done at
> > >      the branch level. This means that when a flow satisfies the filter,
> > >      the entire sub-tree leading to that flow is shown.
> > >
> > > Additional common features:
> > >    - Styling: Styles for both console and HTML formats can be defined
> > >      using a configuration file.
> > >    - Heat maps: Some formats support heat-maps. A color pallete ranging
> > >      from blue (cold) to red (hot) is used to print the number of
> > >      packets and bytes of the flows. That way, the flows that are
> > >      handling more traffic are much easier to spot
> > >
> > > --
> > > V3 -> V4:
> > >  - Add manpage to rpm package
> > >  - Fix Eelco's comments
> > > V2 -> V3:
> > >  - Fix grammar thanks to Eelco's review
> > > V1 -> V2:
> > >  - Fix typos and nits on documentation
> > >  - Split documentation in two: ovs-flowviz.8 man page and a topic
> > >    article with more detailed examples.
> > > RFC -> V1:
> > >  - Addressed Eelco's comments
> > >  - Added a documentation page
> > >  - Added support for dark style HTML pages
> > >  - Patch 3. Small fix in the way a style is looked up. Use the key in
> > >    the KV instead of the metadata string. This helps with "free" values
> > >    such as "output".
> > >
> > > Adrian Moreno (12):
> > >   python: ovs: Add flowviz scheleton.
> > >   python: ovs: flowviz: Add file processing infra.
> > >   python: ovs: flowviz: Add console formatting.
> > >   python: ovs: flowviz: Add default config file.
> > >   python: ovs: flowviz: Add html formatting.
> > >   python: ovs: flowviz: Add datapath tree format.
> > >   python: ovs: flowviz: Add OpenFlow logical view.
> > >   python: ovs: flowviz: Add Openflow cookie format.
> > >   python: ovs: flowviz: Add datapath html format.
> > >   python: ovs: flowviz: Add datapath graph format.
> > >   python: ovs: flowviz: Support html dark style.
> > >   documentation: Document ovs-flowviz.
> > >
> > >  Documentation/automake.mk                   |   4 +-
> > >  Documentation/conf.py                       |   2 +
> > >  Documentation/ref/index.rst                 |   1 +
> > >  Documentation/ref/ovs-flowviz.8.rst         | 531 ++++++++++++++++++++
> > >  Documentation/topics/flow-visualization.rst | 271 ++++++++++
> > >  Documentation/topics/index.rst              |   1 +
> > >  python/automake.mk                          |  32 +-
> > >  python/ovs/flowviz/__init__.py              |   2 +
> > >  python/ovs/flowviz/console.py               | 196 ++++++++
> > >  python/ovs/flowviz/format.py                | 371 ++++++++++++++
> > >  python/ovs/flowviz/html_format.py           | 138 +++++
> > >  python/ovs/flowviz/main.py                  | 196 ++++++++
> > >  python/ovs/flowviz/odp/__init__.py          |   0
> > >  python/ovs/flowviz/odp/cli.py               | 116 +++++
> > >  python/ovs/flowviz/odp/graph.py             | 418 +++++++++++++++
> > >  python/ovs/flowviz/odp/html.py              | 279 ++++++++++
> > >  python/ovs/flowviz/odp/tree.py              | 303 +++++++++++
> > >  python/ovs/flowviz/ofp/__init__.py          |   0
> > >  python/ovs/flowviz/ofp/cli.py               | 246 +++++++++
> > >  python/ovs/flowviz/ofp/html.py              |  98 ++++
> > >  python/ovs/flowviz/ofp/logic.py             | 364 ++++++++++++++
> > >  python/ovs/flowviz/ovs-flowviz              |  20 +
> > >  python/ovs/flowviz/ovs-flowviz.conf         | 128 +++++
> > >  python/ovs/flowviz/process.py               | 263 ++++++++++
> > >  python/setup.py                             |  14 +-
> > >  rhel/openvswitch-fedora.spec.in             |   1 +
> > >  rhel/openvswitch.spec.in                    |   1 +
> > >  27 files changed, 3986 insertions(+), 10 deletions(-)
> > >  create mode 100644 Documentation/ref/ovs-flowviz.8.rst
> > >  create mode 100644 Documentation/topics/flow-visualization.rst
> > >  create mode 100644 python/ovs/flowviz/__init__.py
> > >  create mode 100644 python/ovs/flowviz/console.py
> > >  create mode 100644 python/ovs/flowviz/format.py
> > >  create mode 100644 python/ovs/flowviz/html_format.py
> > >  create mode 100644 python/ovs/flowviz/main.py
> > >  create mode 100644 python/ovs/flowviz/odp/__init__.py
> > >  create mode 100644 python/ovs/flowviz/odp/cli.py
> > >  create mode 100644 python/ovs/flowviz/odp/graph.py
> > >  create mode 100644 python/ovs/flowviz/odp/html.py
> > >  create mode 100644 python/ovs/flowviz/odp/tree.py
> > >  create mode 100644 python/ovs/flowviz/ofp/__init__.py
> > >  create mode 100644 python/ovs/flowviz/ofp/cli.py
> > >  create mode 100644 python/ovs/flowviz/ofp/html.py
> > >  create mode 100644 python/ovs/flowviz/ofp/logic.py
> > >  create mode 100755 python/ovs/flowviz/ovs-flowviz
> > >  create mode 100644 python/ovs/flowviz/ovs-flowviz.conf
> > >  create mode 100644 python/ovs/flowviz/process.py
> > >
> >
> > Hi, Adrian.  Thanks for v4!
> >
> > Not a full review, but I was trying to use this version of flowviz today
> > on a real flow dump from an ovn-kubernetes cluster and it crashes unable
> > to parse check_pkt_len datapath actions:
> >
> > # ovs-flowviz -i snippet1.txt datapath tree
> >
> > Traceback (most recent call last):
> >   File "/python/ovs/flow/kv.py", line 294, in parse
> >     key, val = self._decoders.decode(keyword, value_str)
> >                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/python/ovs/flow/kv.py", line 150, in decode
> >     raise ParseError(
> > ovs.flow.kv.ParseError: Cannot parse key check_pkt_len: No decoder found
> >
> > The above exception was the direct cause of the following exception:
> >
> > Traceback (most recent call last):
> >   File "/python/ovs/flow/kv.py", line 294, in parse
> >     key, val = self._decoders.decode(keyword, value_str)
> >                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/python/ovs/flow/kv.py", line 137, in decode
> >     result = decoder(value_str)
> >              ^^^^^^^^^^^^^^^^^^
> >   File "/python/ovs/flow/kv.py", line 347, in decode_nested_kv_list
> >     parser.parse()
> >   File "/python/ovs/flow/kv.py", line 296, in parse
> >     raise ParseError(
> > ovs.flow.kv.ParseError: Error parsing key-value (check_pkt_len, size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,c
> > ontinuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204b07e)))
> >
> > The above exception was the direct cause of the following exception:
> >
> > The above exception was the direct cause of the following exception:
> >
> > Traceback (most recent call last):
> >   File "/python/ovs/flow/kv.py", line 294, in parse
> >     key, val = self._decoders.decode(keyword, value_str)
> >                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/python/ovs/flow/kv.py", line 137, in decode
> >     result = decoder(value_str)
> >              ^^^^^^^^^^^^^^^^^^
> >   File "/python/ovs/flow/kv.py", line 330, in decode_nested_kv
> >     parser.parse()
> >   File "/python/ovs/flow/kv.py", line 296, in parse
> >     raise ParseError(
> > ovs.flow.kv.ParseError: Error parsing key-value (le, set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,
> > actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(z
> > one=12,nat),recirc(0x204b07e))))
> >
> > The above exception was the direct cause of the following exception:
> >
> > Traceback (most recent call last):
> >   File "/venv/bin/ovs-flowviz", line 20, in <module>
> >     main.main()
> >   File "/python/ovs/flowviz/main.py", line 196, in main
> >     maincli()
> >   File "/venv/lib64/python3.12/site-packages/click/core.py", line 1157, in __call__
> >     return self.main(*args, **kwargs)
> >            ^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/venv/lib64/python3.12/site-packages/click/core.py", line 1078, in main
> >     rv = self.invoke(ctx)
> >          ^^^^^^^^^^^^^^^^
> > File "/venv/lib64/python3.12/site-packages/click/core.py", line 1688, in invoke
> >     return _process_result(sub_ctx.command.invoke(sub_ctx))
> >                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/venv/lib64/python3.12/site-packages/click/core.py", line 1688, in invoke
> >     return _process_result(sub_ctx.command.invoke(sub_ctx))
> >                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/venv/lib64/python3.12/site-packages/click/core.py", line 1434, in invoke
> >     return ctx.invoke(self.callback, **ctx.params)
> >            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/venv/lib64/python3.12/site-packages/click/core.py", line 783, in invoke
> >     return __callback(*args, **kwargs)
> >            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/venv/lib64/python3.12/site-packages/click/decorators.py", line 45, in new_func
> >     return f(get_current_context().obj, *args, **kwargs)
> >            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/python/ovs/flowviz/odp/cli.py", line 85, in tree
> >     processor.process()
> >   File "/python/ovs/flowviz/odp/tree.py", line 233, in process
> >     super().process(False)
> >   File "/python/ovs/flowviz/process.py", line 113, in process
> >     flow = self.create_flow(line, idx)
> >            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >   File "/python/ovs/flowviz/process.py", line 158, in create_flow
> >     return ODPFlow(line, idx)
> >            ^^^^^^^^^^^^^^^^^^
> >   File "/python/ovs/flow/odp.py", line 153, in __init__
> >     aparser.parse()
> >   File "/python/ovs/flow/kv.py", line 296, in parse
> >     raise ParseError(
> > ovs.flow.kv.ParseError: Error parsing key-value (check_pkt_len, size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861757,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204b07e)))))
> >
> >
> > Not the exact same line, but a similar crash happens while parsing:
> >
> > recirc_id(0x204ae73),in_port(20),ct_state(+new-est-rel-rpl-inv+trk),ct_mark(0/0xffff000f),eth(src=1a:1b:1c:1d:18:18,dst=2a:2b:2c:2d:2e:2f),eth_type(0x0800),ipv4(src=192.168.10.1/255.255.255.254,dst=10.64.0.0/255.192.0.0,proto=6,ttl=64,frag=no), packets:7, bytes:518, used:9.077s, flags:S, actions:ct(commit,zone=269,mark=0/0x1,nat(src)),set(eth(src=3a:3b:3c:3d:3e:3f,dst=4a:4b:4c:4d:4e:4f)),set(ipv4(ttl=63)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861236,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204ae75)))))
> >
> > My guess is that it has some trouble with nested check_pkt_len.
> >
>
> Thanks. I'll take a look.
>

If you still have the original flowdump handy, could you please try this
patch?

---
diff --git a/python/ovs/flow/odp.py b/python/ovs/flow/odp.py
index 7d9b165d4..a8f8c067a 100644
--- a/python/ovs/flow/odp.py
+++ b/python/ovs/flow/odp.py
@@ -365,29 +365,30 @@ class ODPFlow(Flow):
             is_list=True,
         )

-        return {
-            **_decoders,
-            "check_pkt_len": nested_kv_decoder(
-                KVDecoders(
-                    {
-                        "size": decode_int,
-                        "gt": nested_kv_decoder(
-                            KVDecoders(
-                                decoders=_decoders,
-                                default_free=decode_free_output,
-                            ),
-                            is_list=True,
+        _decoders["check_pkt_len"] = nested_kv_decoder(
+            KVDecoders(
+                {
+                    "size": decode_int,
+                    "gt": nested_kv_decoder(
+                        KVDecoders(
+                            decoders=_decoders,
+                            default_free=decode_free_output,
                         ),
-                        "le": nested_kv_decoder(
-                            KVDecoders(
-                                decoders=_decoders,
-                                default_free=decode_free_output,
-                            ),
-                            is_list=True,
+                        is_list=True,
+                    ),
+                    "le": nested_kv_decoder(
+                        KVDecoders(
+                            decoders=_decoders,
+                            default_free=decode_free_output,
                         ),
-                    }
-                )
-            ),
+                        is_list=True,
+                    ),
+                }
+            )
+        )
+
+        return {
+            **_decoders,
         }

     @staticmethod
Ilya Maximets May 21, 2024, 6:03 p.m. UTC | #4
On 5/21/24 18:13, amorenoz@redhat.com wrote:
> On Tue, May 21, 2024 at 03:49:03PM GMT, amorenoz@redhat.com wrote:
>> On Tue, May 21, 2024 at 01:51:33PM GMT, Ilya Maximets wrote:
>>> On 5/7/24 16:30, Adrian Moreno wrote:
>>>> The goal of this utility is to read both datapath and Openflow flows
>>>> (using the flow library already available) and print them in different
>>>> formats and styles to make it easier to understand them and troubleshoot
>>>> issues.
>>>>
>>>> The formats are quite opinionated and so are the colors chosen so I'm
>>>> eager to hear what is the impression caused to the community.
>>>>
>>>> Here is a summary of the formats and features supported:
>>>>
>>>> - Openflow
>>>>    - Console: Prints flows back to the console allowing filtering and
>>>>      extensive formatting.
>>>>    - Cookie: Arranges flows based on cookie (instead of table) to ease
>>>>      visualization of OVN-generated flows.
>>>>    - HTML: Prints an HTML file that shows the flows arranged by tables.
>>>>      resubmit actions have a hyperlinke to the target table to ease
>>>>      navegation.
>>>>    - Logic: Many times OVN generates many "logically-equivalent" flows.
>>>>      These are flows that have the same structure: match on the same
>>>>      values and have the same actions. The only thing that changes is
>>>>      the actual value they match against and maybe the arguments of the
>>>>      actions. This format collapses these flows so you can visualize the
>>>>      logical pipeline easier.
>>>>    - JSON: JSON format.
>>>>
>>>> More OpenFlow features:
>>>>    - OVN metadata: Both the "cookie" and the "logic" format allow to
>>>>      connect to a running OVN NB/SB databases and enrich the flows with
>>>>      OVN context based on the flow's cookie.
>>>>
>>>> - Datapath:
>>>>    - Console: Prints flows back to the console allowing filtering and
>>>>      extensive formatting.
>>>>    - Tree: Datapath flows use recirculation to further execute
>>>>      additional actions. This format builds a tree of flows following
>>>>      the recirculation identifiers and prints it in the console.
>>>>    - HTML: Prints the datapath flow tree in HTML. It includes some
>>>>      minimal JS to support expanding and collapsing of entire branches.
>>>>    - Graph: Following the "tree" format, this one prints the tree in
>>>>      graphviz format.
>>>>    - JSON: JSON format.
>>>>
>>>> Additional datapath features:
>>>>    - Many datapath formats are based on the tree flow hierarchy. An
>>>>      interesting feature of this structure is that filtering is done at
>>>>      the branch level. This means that when a flow satisfies the filter,
>>>>      the entire sub-tree leading to that flow is shown.
>>>>
>>>> Additional common features:
>>>>    - Styling: Styles for both console and HTML formats can be defined
>>>>      using a configuration file.
>>>>    - Heat maps: Some formats support heat-maps. A color pallete ranging
>>>>      from blue (cold) to red (hot) is used to print the number of
>>>>      packets and bytes of the flows. That way, the flows that are
>>>>      handling more traffic are much easier to spot
>>>>
>>>> --
>>>> V3 -> V4:
>>>>  - Add manpage to rpm package
>>>>  - Fix Eelco's comments
>>>> V2 -> V3:
>>>>  - Fix grammar thanks to Eelco's review
>>>> V1 -> V2:
>>>>  - Fix typos and nits on documentation
>>>>  - Split documentation in two: ovs-flowviz.8 man page and a topic
>>>>    article with more detailed examples.
>>>> RFC -> V1:
>>>>  - Addressed Eelco's comments
>>>>  - Added a documentation page
>>>>  - Added support for dark style HTML pages
>>>>  - Patch 3. Small fix in the way a style is looked up. Use the key in
>>>>    the KV instead of the metadata string. This helps with "free" values
>>>>    such as "output".
>>>>
>>>> Adrian Moreno (12):
>>>>   python: ovs: Add flowviz scheleton.
>>>>   python: ovs: flowviz: Add file processing infra.
>>>>   python: ovs: flowviz: Add console formatting.
>>>>   python: ovs: flowviz: Add default config file.
>>>>   python: ovs: flowviz: Add html formatting.
>>>>   python: ovs: flowviz: Add datapath tree format.
>>>>   python: ovs: flowviz: Add OpenFlow logical view.
>>>>   python: ovs: flowviz: Add Openflow cookie format.
>>>>   python: ovs: flowviz: Add datapath html format.
>>>>   python: ovs: flowviz: Add datapath graph format.
>>>>   python: ovs: flowviz: Support html dark style.
>>>>   documentation: Document ovs-flowviz.
>>>>
>>>>  Documentation/automake.mk                   |   4 +-
>>>>  Documentation/conf.py                       |   2 +
>>>>  Documentation/ref/index.rst                 |   1 +
>>>>  Documentation/ref/ovs-flowviz.8.rst         | 531 ++++++++++++++++++++
>>>>  Documentation/topics/flow-visualization.rst | 271 ++++++++++
>>>>  Documentation/topics/index.rst              |   1 +
>>>>  python/automake.mk                          |  32 +-
>>>>  python/ovs/flowviz/__init__.py              |   2 +
>>>>  python/ovs/flowviz/console.py               | 196 ++++++++
>>>>  python/ovs/flowviz/format.py                | 371 ++++++++++++++
>>>>  python/ovs/flowviz/html_format.py           | 138 +++++
>>>>  python/ovs/flowviz/main.py                  | 196 ++++++++
>>>>  python/ovs/flowviz/odp/__init__.py          |   0
>>>>  python/ovs/flowviz/odp/cli.py               | 116 +++++
>>>>  python/ovs/flowviz/odp/graph.py             | 418 +++++++++++++++
>>>>  python/ovs/flowviz/odp/html.py              | 279 ++++++++++
>>>>  python/ovs/flowviz/odp/tree.py              | 303 +++++++++++
>>>>  python/ovs/flowviz/ofp/__init__.py          |   0
>>>>  python/ovs/flowviz/ofp/cli.py               | 246 +++++++++
>>>>  python/ovs/flowviz/ofp/html.py              |  98 ++++
>>>>  python/ovs/flowviz/ofp/logic.py             | 364 ++++++++++++++
>>>>  python/ovs/flowviz/ovs-flowviz              |  20 +
>>>>  python/ovs/flowviz/ovs-flowviz.conf         | 128 +++++
>>>>  python/ovs/flowviz/process.py               | 263 ++++++++++
>>>>  python/setup.py                             |  14 +-
>>>>  rhel/openvswitch-fedora.spec.in             |   1 +
>>>>  rhel/openvswitch.spec.in                    |   1 +
>>>>  27 files changed, 3986 insertions(+), 10 deletions(-)
>>>>  create mode 100644 Documentation/ref/ovs-flowviz.8.rst
>>>>  create mode 100644 Documentation/topics/flow-visualization.rst
>>>>  create mode 100644 python/ovs/flowviz/__init__.py
>>>>  create mode 100644 python/ovs/flowviz/console.py
>>>>  create mode 100644 python/ovs/flowviz/format.py
>>>>  create mode 100644 python/ovs/flowviz/html_format.py
>>>>  create mode 100644 python/ovs/flowviz/main.py
>>>>  create mode 100644 python/ovs/flowviz/odp/__init__.py
>>>>  create mode 100644 python/ovs/flowviz/odp/cli.py
>>>>  create mode 100644 python/ovs/flowviz/odp/graph.py
>>>>  create mode 100644 python/ovs/flowviz/odp/html.py
>>>>  create mode 100644 python/ovs/flowviz/odp/tree.py
>>>>  create mode 100644 python/ovs/flowviz/ofp/__init__.py
>>>>  create mode 100644 python/ovs/flowviz/ofp/cli.py
>>>>  create mode 100644 python/ovs/flowviz/ofp/html.py
>>>>  create mode 100644 python/ovs/flowviz/ofp/logic.py
>>>>  create mode 100755 python/ovs/flowviz/ovs-flowviz
>>>>  create mode 100644 python/ovs/flowviz/ovs-flowviz.conf
>>>>  create mode 100644 python/ovs/flowviz/process.py
>>>>
>>>
>>> Hi, Adrian.  Thanks for v4!
>>>
>>> Not a full review, but I was trying to use this version of flowviz today
>>> on a real flow dump from an ovn-kubernetes cluster and it crashes unable
>>> to parse check_pkt_len datapath actions:
>>>
>>> # ovs-flowviz -i snippet1.txt datapath tree
>>>
>>> Traceback (most recent call last):
>>>   File "/python/ovs/flow/kv.py", line 294, in parse
>>>     key, val = self._decoders.decode(keyword, value_str)
>>>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>   File "/python/ovs/flow/kv.py", line 150, in decode
>>>     raise ParseError(
>>> ovs.flow.kv.ParseError: Cannot parse key check_pkt_len: No decoder found

<snip>

>>>
>>> Not the exact same line, but a similar crash happens while parsing:
>>>
>>> recirc_id(0x204ae73),in_port(20),ct_state(+new-est-rel-rpl-inv+trk),ct_mark(0/0xffff000f),eth(src=1a:1b:1c:1d:18:18,dst=2a:2b:2c:2d:2e:2f),eth_type(0x0800),ipv4(src=192.168.10.1/255.255.255.254,dst=10.64.0.0/255.192.0.0,proto=6,ttl=64,frag=no), packets:7, bytes:518, used:9.077s, flags:S, actions:ct(commit,zone=269,mark=0/0x1,nat(src)),set(eth(src=3a:3b:3c:3d:3e:3f,dst=4a:4b:4c:4d:4e:4f)),set(ipv4(ttl=63)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861236,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204ae75)))))
>>>
>>> My guess is that it has some trouble with nested check_pkt_len.
>>>
>>
>> Thanks. I'll take a look.
>>
> 
> If you still have the original flowdump handy, could you please try this
> patch?
> 
> ---
> diff --git a/python/ovs/flow/odp.py b/python/ovs/flow/odp.py
> index 7d9b165d4..a8f8c067a 100644
> --- a/python/ovs/flow/odp.py
> +++ b/python/ovs/flow/odp.py
> @@ -365,29 +365,30 @@ class ODPFlow(Flow):

<snip>

Yeah, that worked.  I'm not sure if the result is more readbale than it was in a plain
text, but it worked. :)

The 'tree' view is a bit confusing because it only takes into account recirculation id
and doesn't care about other fields being completely different.  So, it draws a tree
with a lot of impossible branches...  Do you think there is a way to fix that?

Best regards, Ilya Maximets.
Ilya Maximets May 21, 2024, 9:56 p.m. UTC | #5
On 5/21/24 20:56, amorenoz@redhat.com wrote:
> On Tue, May 21, 2024 at 08:03:49PM GMT, Ilya Maximets wrote:
>> On 5/21/24 18:13, amorenoz@redhat.com wrote:
>>> On Tue, May 21, 2024 at 03:49:03PM GMT, amorenoz@redhat.com wrote:
>>>> On Tue, May 21, 2024 at 01:51:33PM GMT, Ilya Maximets wrote:
>>>>> On 5/7/24 16:30, Adrian Moreno wrote:
>>>>>> The goal of this utility is to read both datapath and Openflow flows
>>>>>> (using the flow library already available) and print them in different
>>>>>> formats and styles to make it easier to understand them and troubleshoot
>>>>>> issues.
>>>>>>
>>>>>> The formats are quite opinionated and so are the colors chosen so I'm
>>>>>> eager to hear what is the impression caused to the community.
>>>>>>
>>>>>> Here is a summary of the formats and features supported:
>>>>>>
>>>>>> - Openflow
>>>>>>    - Console: Prints flows back to the console allowing filtering and
>>>>>>      extensive formatting.
>>>>>>    - Cookie: Arranges flows based on cookie (instead of table) to ease
>>>>>>      visualization of OVN-generated flows.
>>>>>>    - HTML: Prints an HTML file that shows the flows arranged by tables.
>>>>>>      resubmit actions have a hyperlinke to the target table to ease
>>>>>>      navegation.
>>>>>>    - Logic: Many times OVN generates many "logically-equivalent" flows.
>>>>>>      These are flows that have the same structure: match on the same
>>>>>>      values and have the same actions. The only thing that changes is
>>>>>>      the actual value they match against and maybe the arguments of the
>>>>>>      actions. This format collapses these flows so you can visualize the
>>>>>>      logical pipeline easier.
>>>>>>    - JSON: JSON format.
>>>>>>
>>>>>> More OpenFlow features:
>>>>>>    - OVN metadata: Both the "cookie" and the "logic" format allow to
>>>>>>      connect to a running OVN NB/SB databases and enrich the flows with
>>>>>>      OVN context based on the flow's cookie.
>>>>>>
>>>>>> - Datapath:
>>>>>>    - Console: Prints flows back to the console allowing filtering and
>>>>>>      extensive formatting.
>>>>>>    - Tree: Datapath flows use recirculation to further execute
>>>>>>      additional actions. This format builds a tree of flows following
>>>>>>      the recirculation identifiers and prints it in the console.
>>>>>>    - HTML: Prints the datapath flow tree in HTML. It includes some
>>>>>>      minimal JS to support expanding and collapsing of entire branches.
>>>>>>    - Graph: Following the "tree" format, this one prints the tree in
>>>>>>      graphviz format.
>>>>>>    - JSON: JSON format.
>>>>>>
>>>>>> Additional datapath features:
>>>>>>    - Many datapath formats are based on the tree flow hierarchy. An
>>>>>>      interesting feature of this structure is that filtering is done at
>>>>>>      the branch level. This means that when a flow satisfies the filter,
>>>>>>      the entire sub-tree leading to that flow is shown.
>>>>>>
>>>>>> Additional common features:
>>>>>>    - Styling: Styles for both console and HTML formats can be defined
>>>>>>      using a configuration file.
>>>>>>    - Heat maps: Some formats support heat-maps. A color pallete ranging
>>>>>>      from blue (cold) to red (hot) is used to print the number of
>>>>>>      packets and bytes of the flows. That way, the flows that are
>>>>>>      handling more traffic are much easier to spot
>>>>>>
>>>>>> --
>>>>>> V3 -> V4:
>>>>>>  - Add manpage to rpm package
>>>>>>  - Fix Eelco's comments
>>>>>> V2 -> V3:
>>>>>>  - Fix grammar thanks to Eelco's review
>>>>>> V1 -> V2:
>>>>>>  - Fix typos and nits on documentation
>>>>>>  - Split documentation in two: ovs-flowviz.8 man page and a topic
>>>>>>    article with more detailed examples.
>>>>>> RFC -> V1:
>>>>>>  - Addressed Eelco's comments
>>>>>>  - Added a documentation page
>>>>>>  - Added support for dark style HTML pages
>>>>>>  - Patch 3. Small fix in the way a style is looked up. Use the key in
>>>>>>    the KV instead of the metadata string. This helps with "free" values
>>>>>>    such as "output".
>>>>>>
>>>>>> Adrian Moreno (12):
>>>>>>   python: ovs: Add flowviz scheleton.
>>>>>>   python: ovs: flowviz: Add file processing infra.
>>>>>>   python: ovs: flowviz: Add console formatting.
>>>>>>   python: ovs: flowviz: Add default config file.
>>>>>>   python: ovs: flowviz: Add html formatting.
>>>>>>   python: ovs: flowviz: Add datapath tree format.
>>>>>>   python: ovs: flowviz: Add OpenFlow logical view.
>>>>>>   python: ovs: flowviz: Add Openflow cookie format.
>>>>>>   python: ovs: flowviz: Add datapath html format.
>>>>>>   python: ovs: flowviz: Add datapath graph format.
>>>>>>   python: ovs: flowviz: Support html dark style.
>>>>>>   documentation: Document ovs-flowviz.
>>>>>>
>>>>>>  Documentation/automake.mk                   |   4 +-
>>>>>>  Documentation/conf.py                       |   2 +
>>>>>>  Documentation/ref/index.rst                 |   1 +
>>>>>>  Documentation/ref/ovs-flowviz.8.rst         | 531 ++++++++++++++++++++
>>>>>>  Documentation/topics/flow-visualization.rst | 271 ++++++++++
>>>>>>  Documentation/topics/index.rst              |   1 +
>>>>>>  python/automake.mk                          |  32 +-
>>>>>>  python/ovs/flowviz/__init__.py              |   2 +
>>>>>>  python/ovs/flowviz/console.py               | 196 ++++++++
>>>>>>  python/ovs/flowviz/format.py                | 371 ++++++++++++++
>>>>>>  python/ovs/flowviz/html_format.py           | 138 +++++
>>>>>>  python/ovs/flowviz/main.py                  | 196 ++++++++
>>>>>>  python/ovs/flowviz/odp/__init__.py          |   0
>>>>>>  python/ovs/flowviz/odp/cli.py               | 116 +++++
>>>>>>  python/ovs/flowviz/odp/graph.py             | 418 +++++++++++++++
>>>>>>  python/ovs/flowviz/odp/html.py              | 279 ++++++++++
>>>>>>  python/ovs/flowviz/odp/tree.py              | 303 +++++++++++
>>>>>>  python/ovs/flowviz/ofp/__init__.py          |   0
>>>>>>  python/ovs/flowviz/ofp/cli.py               | 246 +++++++++
>>>>>>  python/ovs/flowviz/ofp/html.py              |  98 ++++
>>>>>>  python/ovs/flowviz/ofp/logic.py             | 364 ++++++++++++++
>>>>>>  python/ovs/flowviz/ovs-flowviz              |  20 +
>>>>>>  python/ovs/flowviz/ovs-flowviz.conf         | 128 +++++
>>>>>>  python/ovs/flowviz/process.py               | 263 ++++++++++
>>>>>>  python/setup.py                             |  14 +-
>>>>>>  rhel/openvswitch-fedora.spec.in             |   1 +
>>>>>>  rhel/openvswitch.spec.in                    |   1 +
>>>>>>  27 files changed, 3986 insertions(+), 10 deletions(-)
>>>>>>  create mode 100644 Documentation/ref/ovs-flowviz.8.rst
>>>>>>  create mode 100644 Documentation/topics/flow-visualization.rst
>>>>>>  create mode 100644 python/ovs/flowviz/__init__.py
>>>>>>  create mode 100644 python/ovs/flowviz/console.py
>>>>>>  create mode 100644 python/ovs/flowviz/format.py
>>>>>>  create mode 100644 python/ovs/flowviz/html_format.py
>>>>>>  create mode 100644 python/ovs/flowviz/main.py
>>>>>>  create mode 100644 python/ovs/flowviz/odp/__init__.py
>>>>>>  create mode 100644 python/ovs/flowviz/odp/cli.py
>>>>>>  create mode 100644 python/ovs/flowviz/odp/graph.py
>>>>>>  create mode 100644 python/ovs/flowviz/odp/html.py
>>>>>>  create mode 100644 python/ovs/flowviz/odp/tree.py
>>>>>>  create mode 100644 python/ovs/flowviz/ofp/__init__.py
>>>>>>  create mode 100644 python/ovs/flowviz/ofp/cli.py
>>>>>>  create mode 100644 python/ovs/flowviz/ofp/html.py
>>>>>>  create mode 100644 python/ovs/flowviz/ofp/logic.py
>>>>>>  create mode 100755 python/ovs/flowviz/ovs-flowviz
>>>>>>  create mode 100644 python/ovs/flowviz/ovs-flowviz.conf
>>>>>>  create mode 100644 python/ovs/flowviz/process.py
>>>>>>
>>>>>
>>>>> Hi, Adrian.  Thanks for v4!
>>>>>
>>>>> Not a full review, but I was trying to use this version of flowviz today
>>>>> on a real flow dump from an ovn-kubernetes cluster and it crashes unable
>>>>> to parse check_pkt_len datapath actions:
>>>>>
>>>>> # ovs-flowviz -i snippet1.txt datapath tree
>>>>>
>>>>> Traceback (most recent call last):
>>>>>   File "/python/ovs/flow/kv.py", line 294, in parse
>>>>>     key, val = self._decoders.decode(keyword, value_str)
>>>>>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>   File "/python/ovs/flow/kv.py", line 150, in decode
>>>>>     raise ParseError(
>>>>> ovs.flow.kv.ParseError: Cannot parse key check_pkt_len: No decoder found
>>
>> <snip>
>>
>>>>>
>>>>> Not the exact same line, but a similar crash happens while parsing:
>>>>>
>>>>> recirc_id(0x204ae73),in_port(20),ct_state(+new-est-rel-rpl-inv+trk),ct_mark(0/0xffff000f),eth(src=1a:1b:1c:1d:18:18,dst=2a:2b:2c:2d:2e:2f),eth_type(0x0800),ipv4(src=192.168.10.1/255.255.255.254,dst=10.64.0.0/255.192.0.0,proto=6,ttl=64,frag=no), packets:7, bytes:518, used:9.077s, flags:S, actions:ct(commit,zone=269,mark=0/0x1,nat(src)),set(eth(src=3a:3b:3c:3d:3e:3f,dst=4a:4b:4c:4d:4e:4f)),set(ipv4(ttl=63)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861236,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204ae75)))))
>>>>>
>>>>> My guess is that it has some trouble with nested check_pkt_len.
>>>>>
>>>>
>>>> Thanks. I'll take a look.
>>>>
>>>
>>> If you still have the original flowdump handy, could you please try this
>>> patch?
>>>
>>> ---
>>> diff --git a/python/ovs/flow/odp.py b/python/ovs/flow/odp.py
>>> index 7d9b165d4..a8f8c067a 100644
>>> --- a/python/ovs/flow/odp.py
>>> +++ b/python/ovs/flow/odp.py
>>> @@ -365,29 +365,30 @@ class ODPFlow(Flow):
>>
>> <snip>
>>
>> Yeah, that worked.  I'm not sure if the result is more readbale than it was in a plain
>> text, but it worked. :)
>>
> 
> Sorry it didn't help.

It's about 500 flows and they are not pretty as seen above.  So, it's hard to
make them look good.

> Do you have any suggestion (apart from the one
> below)?
> 
>> The 'tree' view is a bit confusing because it only takes into account recirculation id
>> and doesn't care about other fields being completely different.  So, it draws a tree
>> with a lot of impossible branches...  Do you think there is a way to fix that?
>>
> 
> Without implementing a matching engine, getting 100% accuracy is tough.
> I added filtering and highlighting to assist but at the end it's true you have
> to mentally do the matching.

Yeah.  In general case for a 100% accuracy we would have to implement parts
of the datapath, e.g. execute the actions.

However, some things are immutable and do not support masked matches.  Some
of these are also always present.  For example, in_port() is always there
and it cannot be changed by actions, so we can avoid trees like this:

│   ├── recirc_id(0x1c43aff),in_port(18)...
│   │   actions:...recirc(0x204b096)
│   │   └── recirc_id(0x204b096),in_port(18),
│   │       actions:set(ipv4(ttl=62)),hash(l4(0)),recirc(0x204b097)
│   │       ├── recirc_id(0x204b097),dp_hash(0x8/0xf),in_port(18)...
│   │       │   actions:...recirc(0x1c2f203)
│   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(23)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
│   │       │   └── recirc_id(0x1c2f203),in_port(18)...
│   │       ├── recirc_id(0x204b097),dp_hash(0x4/0xf),in_port(18)
│   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(23)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
│   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
│   │       │   └── recirc_id(0x1c2f203),in_port(18)...


Above is heavily abridged, but yo can see that only 4 out of 18 flows
at the last level match on the same input port.  This may be optimized.

In general it just blows out the total output from 500 flows into 18000
lines of a tree output in my case.

There are trees where on some level many flows have exact same actions
with slightly different matches.  I wonder if these can be groupped to
only list the actions once.  This is beneficial especially if those
actions have reirculation in them, so it will also not be repeated.

> 
> I also thought of having some interactivity (I added some very basic one
> in the html format) so you could manually remove the branches you're not
> interested in. But that's the only thin

Yeah, I'd really like interactive filtering... maybe even not interactive for
the starters.  Maybe just a string match or regex instead of packet-aware
filtering.  The way it should work is to go through the tree and print out
only paths that contain a match, i.e. all parents of the matching flow
and children.  Basically, cut out the siblings.  For exmaple, something like
--filter="in_port(18)" should only print trees that match on port 18.
This is an easy one, since input port is always in the flow.

A big problem I'm also facing is that grep doesn't work due to coloring.
And if I remove the colors for grep I will not be able to add them back,
so grep filtering kind of has to be supported natively in ovs-flowviz.

All in all, what we all want is just have retis print out datapath flows as
packets match them. :D  So, we can pcap-filter a TCP stream and see all
the datapath flows it matched and actions executed.

Best regards, Ilya Maximets.

> 
> BTW. Thanks for trying it out and giving feedback.
> 
> --
> Adrián
>
Ilya Maximets May 21, 2024, 10:17 p.m. UTC | #6
On 5/21/24 23:56, Ilya Maximets wrote:
> On 5/21/24 20:56, amorenoz@redhat.com wrote:
>> On Tue, May 21, 2024 at 08:03:49PM GMT, Ilya Maximets wrote:
>>> On 5/21/24 18:13, amorenoz@redhat.com wrote:
>>>> On Tue, May 21, 2024 at 03:49:03PM GMT, amorenoz@redhat.com wrote:
>>>>> On Tue, May 21, 2024 at 01:51:33PM GMT, Ilya Maximets wrote:
>>>>>> On 5/7/24 16:30, Adrian Moreno wrote:
>>>>>>> The goal of this utility is to read both datapath and Openflow flows
>>>>>>> (using the flow library already available) and print them in different
>>>>>>> formats and styles to make it easier to understand them and troubleshoot
>>>>>>> issues.
>>>>>>>
>>>>>>> The formats are quite opinionated and so are the colors chosen so I'm
>>>>>>> eager to hear what is the impression caused to the community.
>>>>>>>
>>>>>>> Here is a summary of the formats and features supported:
>>>>>>>
>>>>>>> - Openflow
>>>>>>>    - Console: Prints flows back to the console allowing filtering and
>>>>>>>      extensive formatting.
>>>>>>>    - Cookie: Arranges flows based on cookie (instead of table) to ease
>>>>>>>      visualization of OVN-generated flows.
>>>>>>>    - HTML: Prints an HTML file that shows the flows arranged by tables.
>>>>>>>      resubmit actions have a hyperlinke to the target table to ease
>>>>>>>      navegation.
>>>>>>>    - Logic: Many times OVN generates many "logically-equivalent" flows.
>>>>>>>      These are flows that have the same structure: match on the same
>>>>>>>      values and have the same actions. The only thing that changes is
>>>>>>>      the actual value they match against and maybe the arguments of the
>>>>>>>      actions. This format collapses these flows so you can visualize the
>>>>>>>      logical pipeline easier.
>>>>>>>    - JSON: JSON format.
>>>>>>>
>>>>>>> More OpenFlow features:
>>>>>>>    - OVN metadata: Both the "cookie" and the "logic" format allow to
>>>>>>>      connect to a running OVN NB/SB databases and enrich the flows with
>>>>>>>      OVN context based on the flow's cookie.
>>>>>>>
>>>>>>> - Datapath:
>>>>>>>    - Console: Prints flows back to the console allowing filtering and
>>>>>>>      extensive formatting.
>>>>>>>    - Tree: Datapath flows use recirculation to further execute
>>>>>>>      additional actions. This format builds a tree of flows following
>>>>>>>      the recirculation identifiers and prints it in the console.
>>>>>>>    - HTML: Prints the datapath flow tree in HTML. It includes some
>>>>>>>      minimal JS to support expanding and collapsing of entire branches.
>>>>>>>    - Graph: Following the "tree" format, this one prints the tree in
>>>>>>>      graphviz format.
>>>>>>>    - JSON: JSON format.
>>>>>>>
>>>>>>> Additional datapath features:
>>>>>>>    - Many datapath formats are based on the tree flow hierarchy. An
>>>>>>>      interesting feature of this structure is that filtering is done at
>>>>>>>      the branch level. This means that when a flow satisfies the filter,
>>>>>>>      the entire sub-tree leading to that flow is shown.
>>>>>>>
>>>>>>> Additional common features:
>>>>>>>    - Styling: Styles for both console and HTML formats can be defined
>>>>>>>      using a configuration file.
>>>>>>>    - Heat maps: Some formats support heat-maps. A color pallete ranging
>>>>>>>      from blue (cold) to red (hot) is used to print the number of
>>>>>>>      packets and bytes of the flows. That way, the flows that are
>>>>>>>      handling more traffic are much easier to spot
>>>>>>>
>>>>>>> --
>>>>>>> V3 -> V4:
>>>>>>>  - Add manpage to rpm package
>>>>>>>  - Fix Eelco's comments
>>>>>>> V2 -> V3:
>>>>>>>  - Fix grammar thanks to Eelco's review
>>>>>>> V1 -> V2:
>>>>>>>  - Fix typos and nits on documentation
>>>>>>>  - Split documentation in two: ovs-flowviz.8 man page and a topic
>>>>>>>    article with more detailed examples.
>>>>>>> RFC -> V1:
>>>>>>>  - Addressed Eelco's comments
>>>>>>>  - Added a documentation page
>>>>>>>  - Added support for dark style HTML pages
>>>>>>>  - Patch 3. Small fix in the way a style is looked up. Use the key in
>>>>>>>    the KV instead of the metadata string. This helps with "free" values
>>>>>>>    such as "output".
>>>>>>>
>>>>>>> Adrian Moreno (12):
>>>>>>>   python: ovs: Add flowviz scheleton.
>>>>>>>   python: ovs: flowviz: Add file processing infra.
>>>>>>>   python: ovs: flowviz: Add console formatting.
>>>>>>>   python: ovs: flowviz: Add default config file.
>>>>>>>   python: ovs: flowviz: Add html formatting.
>>>>>>>   python: ovs: flowviz: Add datapath tree format.
>>>>>>>   python: ovs: flowviz: Add OpenFlow logical view.
>>>>>>>   python: ovs: flowviz: Add Openflow cookie format.
>>>>>>>   python: ovs: flowviz: Add datapath html format.
>>>>>>>   python: ovs: flowviz: Add datapath graph format.
>>>>>>>   python: ovs: flowviz: Support html dark style.
>>>>>>>   documentation: Document ovs-flowviz.
>>>>>>>
>>>>>>>  Documentation/automake.mk                   |   4 +-
>>>>>>>  Documentation/conf.py                       |   2 +
>>>>>>>  Documentation/ref/index.rst                 |   1 +
>>>>>>>  Documentation/ref/ovs-flowviz.8.rst         | 531 ++++++++++++++++++++
>>>>>>>  Documentation/topics/flow-visualization.rst | 271 ++++++++++
>>>>>>>  Documentation/topics/index.rst              |   1 +
>>>>>>>  python/automake.mk                          |  32 +-
>>>>>>>  python/ovs/flowviz/__init__.py              |   2 +
>>>>>>>  python/ovs/flowviz/console.py               | 196 ++++++++
>>>>>>>  python/ovs/flowviz/format.py                | 371 ++++++++++++++
>>>>>>>  python/ovs/flowviz/html_format.py           | 138 +++++
>>>>>>>  python/ovs/flowviz/main.py                  | 196 ++++++++
>>>>>>>  python/ovs/flowviz/odp/__init__.py          |   0
>>>>>>>  python/ovs/flowviz/odp/cli.py               | 116 +++++
>>>>>>>  python/ovs/flowviz/odp/graph.py             | 418 +++++++++++++++
>>>>>>>  python/ovs/flowviz/odp/html.py              | 279 ++++++++++
>>>>>>>  python/ovs/flowviz/odp/tree.py              | 303 +++++++++++
>>>>>>>  python/ovs/flowviz/ofp/__init__.py          |   0
>>>>>>>  python/ovs/flowviz/ofp/cli.py               | 246 +++++++++
>>>>>>>  python/ovs/flowviz/ofp/html.py              |  98 ++++
>>>>>>>  python/ovs/flowviz/ofp/logic.py             | 364 ++++++++++++++
>>>>>>>  python/ovs/flowviz/ovs-flowviz              |  20 +
>>>>>>>  python/ovs/flowviz/ovs-flowviz.conf         | 128 +++++
>>>>>>>  python/ovs/flowviz/process.py               | 263 ++++++++++
>>>>>>>  python/setup.py                             |  14 +-
>>>>>>>  rhel/openvswitch-fedora.spec.in             |   1 +
>>>>>>>  rhel/openvswitch.spec.in                    |   1 +
>>>>>>>  27 files changed, 3986 insertions(+), 10 deletions(-)
>>>>>>>  create mode 100644 Documentation/ref/ovs-flowviz.8.rst
>>>>>>>  create mode 100644 Documentation/topics/flow-visualization.rst
>>>>>>>  create mode 100644 python/ovs/flowviz/__init__.py
>>>>>>>  create mode 100644 python/ovs/flowviz/console.py
>>>>>>>  create mode 100644 python/ovs/flowviz/format.py
>>>>>>>  create mode 100644 python/ovs/flowviz/html_format.py
>>>>>>>  create mode 100644 python/ovs/flowviz/main.py
>>>>>>>  create mode 100644 python/ovs/flowviz/odp/__init__.py
>>>>>>>  create mode 100644 python/ovs/flowviz/odp/cli.py
>>>>>>>  create mode 100644 python/ovs/flowviz/odp/graph.py
>>>>>>>  create mode 100644 python/ovs/flowviz/odp/html.py
>>>>>>>  create mode 100644 python/ovs/flowviz/odp/tree.py
>>>>>>>  create mode 100644 python/ovs/flowviz/ofp/__init__.py
>>>>>>>  create mode 100644 python/ovs/flowviz/ofp/cli.py
>>>>>>>  create mode 100644 python/ovs/flowviz/ofp/html.py
>>>>>>>  create mode 100644 python/ovs/flowviz/ofp/logic.py
>>>>>>>  create mode 100755 python/ovs/flowviz/ovs-flowviz
>>>>>>>  create mode 100644 python/ovs/flowviz/ovs-flowviz.conf
>>>>>>>  create mode 100644 python/ovs/flowviz/process.py
>>>>>>>
>>>>>>
>>>>>> Hi, Adrian.  Thanks for v4!
>>>>>>
>>>>>> Not a full review, but I was trying to use this version of flowviz today
>>>>>> on a real flow dump from an ovn-kubernetes cluster and it crashes unable
>>>>>> to parse check_pkt_len datapath actions:
>>>>>>
>>>>>> # ovs-flowviz -i snippet1.txt datapath tree
>>>>>>
>>>>>> Traceback (most recent call last):
>>>>>>   File "/python/ovs/flow/kv.py", line 294, in parse
>>>>>>     key, val = self._decoders.decode(keyword, value_str)
>>>>>>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>   File "/python/ovs/flow/kv.py", line 150, in decode
>>>>>>     raise ParseError(
>>>>>> ovs.flow.kv.ParseError: Cannot parse key check_pkt_len: No decoder found
>>>
>>> <snip>
>>>
>>>>>>
>>>>>> Not the exact same line, but a similar crash happens while parsing:
>>>>>>
>>>>>> recirc_id(0x204ae73),in_port(20),ct_state(+new-est-rel-rpl-inv+trk),ct_mark(0/0xffff000f),eth(src=1a:1b:1c:1d:18:18,dst=2a:2b:2c:2d:2e:2f),eth_type(0x0800),ipv4(src=192.168.10.1/255.255.255.254,dst=10.64.0.0/255.192.0.0,proto=6,ttl=64,frag=no), packets:7, bytes:518, used:9.077s, flags:S, actions:ct(commit,zone=269,mark=0/0x1,nat(src)),set(eth(src=3a:3b:3c:3d:3e:3f,dst=4a:4b:4c:4d:4e:4f)),set(ipv4(ttl=63)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861236,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204ae75)))))
>>>>>>
>>>>>> My guess is that it has some trouble with nested check_pkt_len.
>>>>>>
>>>>>
>>>>> Thanks. I'll take a look.
>>>>>
>>>>
>>>> If you still have the original flowdump handy, could you please try this
>>>> patch?
>>>>
>>>> ---
>>>> diff --git a/python/ovs/flow/odp.py b/python/ovs/flow/odp.py
>>>> index 7d9b165d4..a8f8c067a 100644
>>>> --- a/python/ovs/flow/odp.py
>>>> +++ b/python/ovs/flow/odp.py
>>>> @@ -365,29 +365,30 @@ class ODPFlow(Flow):
>>>
>>> <snip>
>>>
>>> Yeah, that worked.  I'm not sure if the result is more readbale than it was in a plain
>>> text, but it worked. :)
>>>
>>
>> Sorry it didn't help.
> 
> It's about 500 flows and they are not pretty as seen above.  So, it's hard to
> make them look good.
> 
>> Do you have any suggestion (apart from the one
>> below)?
>>
>>> The 'tree' view is a bit confusing because it only takes into account recirculation id
>>> and doesn't care about other fields being completely different.  So, it draws a tree
>>> with a lot of impossible branches...  Do you think there is a way to fix that?
>>>
>>
>> Without implementing a matching engine, getting 100% accuracy is tough.
>> I added filtering and highlighting to assist but at the end it's true you have
>> to mentally do the matching.
> 
> Yeah.  In general case for a 100% accuracy we would have to implement parts
> of the datapath, e.g. execute the actions.
> 
> However, some things are immutable and do not support masked matches.  Some
> of these are also always present.  For example, in_port() is always there
> and it cannot be changed by actions, so we can avoid trees like this:
> 
> │   ├── recirc_id(0x1c43aff),in_port(18)...
> │   │   actions:...recirc(0x204b096)
> │   │   └── recirc_id(0x204b096),in_port(18),
> │   │       actions:set(ipv4(ttl=62)),hash(l4(0)),recirc(0x204b097)
> │   │       ├── recirc_id(0x204b097),dp_hash(0x8/0xf),in_port(18)...
> │   │       │   actions:...recirc(0x1c2f203)
> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(23)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
> │   │       │   └── recirc_id(0x1c2f203),in_port(18)...
> │   │       ├── recirc_id(0x204b097),dp_hash(0x4/0xf),in_port(18)
> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(23)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
> │   │       │   └── recirc_id(0x1c2f203),in_port(18)...
> 
> 
> Above is heavily abridged, but yo can see that only 4 out of 18 flows
> at the last level match on the same input port.  This may be optimized.
> 
> In general it just blows out the total output from 500 flows into 18000
> lines of a tree output in my case.

Correction: this is related to repetitions in general, removing flows
based on in_port alone will reduce the 18K number, but it will sill be
way far from 500.

> 
> There are trees where on some level many flows have exact same actions
> with slightly different matches.  I wonder if these can be groupped to
> only list the actions once.  This is beneficial especially if those
> actions have reirculation in them, so it will also not be repeated.
> 
>>
>> I also thought of having some interactivity (I added some very basic one
>> in the html format) so you could manually remove the branches you're not
>> interested in. But that's the only thin
> 
> Yeah, I'd really like interactive filtering... maybe even not interactive for
> the starters.  Maybe just a string match or regex instead of packet-aware
> filtering.  The way it should work is to go through the tree and print out
> only paths that contain a match, i.e. all parents of the matching flow
> and children.  Basically, cut out the siblings.  For exmaple, something like
> --filter="in_port(18)" should only print trees that match on port 18.
> This is an easy one, since input port is always in the flow.
> 
> A big problem I'm also facing is that grep doesn't work due to coloring.
> And if I remove the colors for grep I will not be able to add them back,
> so grep filtering kind of has to be supported natively in ovs-flowviz.
> 
> All in all, what we all want is just have retis print out datapath flows as
> packets match them. :D  So, we can pcap-filter a TCP stream and see all
> the datapath flows it matched and actions executed.
> 
> Best regards, Ilya Maximets.
> 
>>
>> BTW. Thanks for trying it out and giving feedback.
>>
>> --
>> Adrián
>>
>
Ilya Maximets May 22, 2024, 11:12 a.m. UTC | #7
Please, keep the list in CC. :)

On 5/22/24 12:52, amorenoz@redhat.com wrote:
> On Wed, May 22, 2024 at 12:17:37AM GMT, Ilya Maximets wrote:
>> On 5/21/24 23:56, Ilya Maximets wrote:
>>> On 5/21/24 20:56, amorenoz@redhat.com wrote:
>>>> On Tue, May 21, 2024 at 08:03:49PM GMT, Ilya Maximets wrote:
>>>>> On 5/21/24 18:13, amorenoz@redhat.com wrote:
>>>>>> On Tue, May 21, 2024 at 03:49:03PM GMT, amorenoz@redhat.com wrote:
>>>>>>> On Tue, May 21, 2024 at 01:51:33PM GMT, Ilya Maximets wrote:
>>>>>>>> On 5/7/24 16:30, Adrian Moreno wrote:
>>>>>>>>> The goal of this utility is to read both datapath and Openflow flows
>>>>>>>>> (using the flow library already available) and print them in different
>>>>>>>>> formats and styles to make it easier to understand them and troubleshoot
>>>>>>>>> issues.
>>>>>>>>>
>>>>>>>>> The formats are quite opinionated and so are the colors chosen so I'm
>>>>>>>>> eager to hear what is the impression caused to the community.
>>>>>>>>>
>>>>>>>>> Here is a summary of the formats and features supported:
>>>>>>>>>
>>>>>>>>> - Openflow
>>>>>>>>>    - Console: Prints flows back to the console allowing filtering and
>>>>>>>>>      extensive formatting.
>>>>>>>>>    - Cookie: Arranges flows based on cookie (instead of table) to ease
>>>>>>>>>      visualization of OVN-generated flows.
>>>>>>>>>    - HTML: Prints an HTML file that shows the flows arranged by tables.
>>>>>>>>>      resubmit actions have a hyperlinke to the target table to ease
>>>>>>>>>      navegation.
>>>>>>>>>    - Logic: Many times OVN generates many "logically-equivalent" flows.
>>>>>>>>>      These are flows that have the same structure: match on the same
>>>>>>>>>      values and have the same actions. The only thing that changes is
>>>>>>>>>      the actual value they match against and maybe the arguments of the
>>>>>>>>>      actions. This format collapses these flows so you can visualize the
>>>>>>>>>      logical pipeline easier.
>>>>>>>>>    - JSON: JSON format.
>>>>>>>>>
>>>>>>>>> More OpenFlow features:
>>>>>>>>>    - OVN metadata: Both the "cookie" and the "logic" format allow to
>>>>>>>>>      connect to a running OVN NB/SB databases and enrich the flows with
>>>>>>>>>      OVN context based on the flow's cookie.
>>>>>>>>>
>>>>>>>>> - Datapath:
>>>>>>>>>    - Console: Prints flows back to the console allowing filtering and
>>>>>>>>>      extensive formatting.
>>>>>>>>>    - Tree: Datapath flows use recirculation to further execute
>>>>>>>>>      additional actions. This format builds a tree of flows following
>>>>>>>>>      the recirculation identifiers and prints it in the console.
>>>>>>>>>    - HTML: Prints the datapath flow tree in HTML. It includes some
>>>>>>>>>      minimal JS to support expanding and collapsing of entire branches.
>>>>>>>>>    - Graph: Following the "tree" format, this one prints the tree in
>>>>>>>>>      graphviz format.
>>>>>>>>>    - JSON: JSON format.
>>>>>>>>>
>>>>>>>>> Additional datapath features:
>>>>>>>>>    - Many datapath formats are based on the tree flow hierarchy. An
>>>>>>>>>      interesting feature of this structure is that filtering is done at
>>>>>>>>>      the branch level. This means that when a flow satisfies the filter,
>>>>>>>>>      the entire sub-tree leading to that flow is shown.
>>>>>>>>>
>>>>>>>>> Additional common features:
>>>>>>>>>    - Styling: Styles for both console and HTML formats can be defined
>>>>>>>>>      using a configuration file.
>>>>>>>>>    - Heat maps: Some formats support heat-maps. A color pallete ranging
>>>>>>>>>      from blue (cold) to red (hot) is used to print the number of
>>>>>>>>>      packets and bytes of the flows. That way, the flows that are
>>>>>>>>>      handling more traffic are much easier to spot
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> V3 -> V4:
>>>>>>>>>  - Add manpage to rpm package
>>>>>>>>>  - Fix Eelco's comments
>>>>>>>>> V2 -> V3:
>>>>>>>>>  - Fix grammar thanks to Eelco's review
>>>>>>>>> V1 -> V2:
>>>>>>>>>  - Fix typos and nits on documentation
>>>>>>>>>  - Split documentation in two: ovs-flowviz.8 man page and a topic
>>>>>>>>>    article with more detailed examples.
>>>>>>>>> RFC -> V1:
>>>>>>>>>  - Addressed Eelco's comments
>>>>>>>>>  - Added a documentation page
>>>>>>>>>  - Added support for dark style HTML pages
>>>>>>>>>  - Patch 3. Small fix in the way a style is looked up. Use the key in
>>>>>>>>>    the KV instead of the metadata string. This helps with "free" values
>>>>>>>>>    such as "output".
>>>>>>>>>
>>>>>>>>> Adrian Moreno (12):
>>>>>>>>>   python: ovs: Add flowviz scheleton.
>>>>>>>>>   python: ovs: flowviz: Add file processing infra.
>>>>>>>>>   python: ovs: flowviz: Add console formatting.
>>>>>>>>>   python: ovs: flowviz: Add default config file.
>>>>>>>>>   python: ovs: flowviz: Add html formatting.
>>>>>>>>>   python: ovs: flowviz: Add datapath tree format.
>>>>>>>>>   python: ovs: flowviz: Add OpenFlow logical view.
>>>>>>>>>   python: ovs: flowviz: Add Openflow cookie format.
>>>>>>>>>   python: ovs: flowviz: Add datapath html format.
>>>>>>>>>   python: ovs: flowviz: Add datapath graph format.
>>>>>>>>>   python: ovs: flowviz: Support html dark style.
>>>>>>>>>   documentation: Document ovs-flowviz.
>>>>>>>>>
>>>>>>>>>  Documentation/automake.mk                   |   4 +-
>>>>>>>>>  Documentation/conf.py                       |   2 +
>>>>>>>>>  Documentation/ref/index.rst                 |   1 +
>>>>>>>>>  Documentation/ref/ovs-flowviz.8.rst         | 531 ++++++++++++++++++++
>>>>>>>>>  Documentation/topics/flow-visualization.rst | 271 ++++++++++
>>>>>>>>>  Documentation/topics/index.rst              |   1 +
>>>>>>>>>  python/automake.mk                          |  32 +-
>>>>>>>>>  python/ovs/flowviz/__init__.py              |   2 +
>>>>>>>>>  python/ovs/flowviz/console.py               | 196 ++++++++
>>>>>>>>>  python/ovs/flowviz/format.py                | 371 ++++++++++++++
>>>>>>>>>  python/ovs/flowviz/html_format.py           | 138 +++++
>>>>>>>>>  python/ovs/flowviz/main.py                  | 196 ++++++++
>>>>>>>>>  python/ovs/flowviz/odp/__init__.py          |   0
>>>>>>>>>  python/ovs/flowviz/odp/cli.py               | 116 +++++
>>>>>>>>>  python/ovs/flowviz/odp/graph.py             | 418 +++++++++++++++
>>>>>>>>>  python/ovs/flowviz/odp/html.py              | 279 ++++++++++
>>>>>>>>>  python/ovs/flowviz/odp/tree.py              | 303 +++++++++++
>>>>>>>>>  python/ovs/flowviz/ofp/__init__.py          |   0
>>>>>>>>>  python/ovs/flowviz/ofp/cli.py               | 246 +++++++++
>>>>>>>>>  python/ovs/flowviz/ofp/html.py              |  98 ++++
>>>>>>>>>  python/ovs/flowviz/ofp/logic.py             | 364 ++++++++++++++
>>>>>>>>>  python/ovs/flowviz/ovs-flowviz              |  20 +
>>>>>>>>>  python/ovs/flowviz/ovs-flowviz.conf         | 128 +++++
>>>>>>>>>  python/ovs/flowviz/process.py               | 263 ++++++++++
>>>>>>>>>  python/setup.py                             |  14 +-
>>>>>>>>>  rhel/openvswitch-fedora.spec.in             |   1 +
>>>>>>>>>  rhel/openvswitch.spec.in                    |   1 +
>>>>>>>>>  27 files changed, 3986 insertions(+), 10 deletions(-)
>>>>>>>>>  create mode 100644 Documentation/ref/ovs-flowviz.8.rst
>>>>>>>>>  create mode 100644 Documentation/topics/flow-visualization.rst
>>>>>>>>>  create mode 100644 python/ovs/flowviz/__init__.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/console.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/format.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/html_format.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/main.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/odp/__init__.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/odp/cli.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/odp/graph.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/odp/html.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/odp/tree.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/ofp/__init__.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/ofp/cli.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/ofp/html.py
>>>>>>>>>  create mode 100644 python/ovs/flowviz/ofp/logic.py
>>>>>>>>>  create mode 100755 python/ovs/flowviz/ovs-flowviz
>>>>>>>>>  create mode 100644 python/ovs/flowviz/ovs-flowviz.conf
>>>>>>>>>  create mode 100644 python/ovs/flowviz/process.py
>>>>>>>>>
>>>>>>>>
>>>>>>>> Hi, Adrian.  Thanks for v4!
>>>>>>>>
>>>>>>>> Not a full review, but I was trying to use this version of flowviz today
>>>>>>>> on a real flow dump from an ovn-kubernetes cluster and it crashes unable
>>>>>>>> to parse check_pkt_len datapath actions:
>>>>>>>>
>>>>>>>> # ovs-flowviz -i snippet1.txt datapath tree
>>>>>>>>
>>>>>>>> Traceback (most recent call last):
>>>>>>>>   File "/python/ovs/flow/kv.py", line 294, in parse
>>>>>>>>     key, val = self._decoders.decode(keyword, value_str)
>>>>>>>>                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>>>   File "/python/ovs/flow/kv.py", line 150, in decode
>>>>>>>>     raise ParseError(
>>>>>>>> ovs.flow.kv.ParseError: Cannot parse key check_pkt_len: No decoder found
>>>>>
>>>>> <snip>
>>>>>
>>>>>>>>
>>>>>>>> Not the exact same line, but a similar crash happens while parsing:
>>>>>>>>
>>>>>>>> recirc_id(0x204ae73),in_port(20),ct_state(+new-est-rel-rpl-inv+trk),ct_mark(0/0xffff000f),eth(src=1a:1b:1c:1d:18:18,dst=2a:2b:2c:2d:2e:2f),eth_type(0x0800),ipv4(src=192.168.10.1/255.255.255.254,dst=10.64.0.0/255.192.0.0,proto=6,ttl=64,frag=no), packets:7, bytes:518, used:9.077s, flags:S, actions:ct(commit,zone=269,mark=0/0x1,nat(src)),set(eth(src=3a:3b:3c:3d:3e:3f,dst=4a:4b:4c:4d:4e:4f)),set(ipv4(ttl=63)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=29551991,rule_cookie=0x769b5340,controller_id=0,max_len=65535))))),le(set(eth(src=0a:0b:0c:0d:08:08,dst=aa:bb:cc:dd:ee:ff)),set(ipv4(ttl=62)),check_pkt_len(size=8514,gt(sample(sample=100.0%,actions(meter(4),userspace(pid=4294967295,controller(reason=1,dont_send=0,continuation=0,recirc_id=33861236,rule_cookie=0x9ac6aa33,controller_id=0,max_len=65535))))),le(ct(zone=12,nat),recirc(0x204ae75)))))
>>>>>>>>
>>>>>>>> My guess is that it has some trouble with nested check_pkt_len.
>>>>>>>>
>>>>>>>
>>>>>>> Thanks. I'll take a look.
>>>>>>>
>>>>>>
>>>>>> If you still have the original flowdump handy, could you please try this
>>>>>> patch?
>>>>>>
>>>>>> ---
>>>>>> diff --git a/python/ovs/flow/odp.py b/python/ovs/flow/odp.py
>>>>>> index 7d9b165d4..a8f8c067a 100644
>>>>>> --- a/python/ovs/flow/odp.py
>>>>>> +++ b/python/ovs/flow/odp.py
>>>>>> @@ -365,29 +365,30 @@ class ODPFlow(Flow):
>>>>>
>>>>> <snip>
>>>>>
>>>>> Yeah, that worked.  I'm not sure if the result is more readbale than it was in a plain
>>>>> text, but it worked. :)
>>>>>
>>>>
>>>> Sorry it didn't help.
>>>
>>> It's about 500 flows and they are not pretty as seen above.  So, it's hard to
>>> make them look good.
>>>
>>>> Do you have any suggestion (apart from the one
>>>> below)?
>>>>
>>>>> The 'tree' view is a bit confusing because it only takes into account recirculation id
>>>>> and doesn't care about other fields being completely different.  So, it draws a tree
>>>>> with a lot of impossible branches...  Do you think there is a way to fix that?
>>>>>
>>>>
>>>> Without implementing a matching engine, getting 100% accuracy is tough.
>>>> I added filtering and highlighting to assist but at the end it's true you have
>>>> to mentally do the matching.
>>>
>>> Yeah.  In general case for a 100% accuracy we would have to implement parts
>>> of the datapath, e.g. execute the actions.
>>>
>>> However, some things are immutable and do not support masked matches.  Some
>>> of these are also always present.  For example, in_port() is always there
>>> and it cannot be changed by actions, so we can avoid trees like this:
>>>
>>> │   ├── recirc_id(0x1c43aff),in_port(18)...
>>> │   │   actions:...recirc(0x204b096)
>>> │   │   └── recirc_id(0x204b096),in_port(18),
>>> │   │       actions:set(ipv4(ttl=62)),hash(l4(0)),recirc(0x204b097)
>>> │   │       ├── recirc_id(0x204b097),dp_hash(0x8/0xf),in_port(18)...
>>> │   │       │   actions:...recirc(0x1c2f203)
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(23)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
>>> │   │       │   └── recirc_id(0x1c2f203),in_port(18)...
>>> │   │       ├── recirc_id(0x204b097),dp_hash(0x4/0xf),in_port(18)
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(23)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(19)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(18)...
>>> │   │       │   ├── recirc_id(0x1c2f203),in_port(14)...
>>> │   │       │   └── recirc_id(0x1c2f203),in_port(18)...
>>>
>>>
>>> Above is heavily abridged, but yo can see that only 4 out of 18 flows
>>> at the last level match on the same input port.  This may be optimized.
>>>
> 
> Good idea! That's a low hanging fruit that should ease things out. I'll
> submit a new version of the series implementing it.

Thanks!  Please, aslo add a test case for parsing the nested check_pkt_len
that caused the crash in this version.

> 
>>> In general it just blows out the total output from 500 flows into 18000
>>> lines of a tree output in my case.
>>
>> Correction: this is related to repetitions in general, removing flows
>> based on in_port alone will reduce the 18K number, but it will sill be
>> way far from 500.
>>
>>>
>>> There are trees where on some level many flows have exact same actions
>>> with slightly different matches.  I wonder if these can be groupped to
>>> only list the actions once.  This is beneficial especially if those
>>> actions have reirculation in them, so it will also not be repeated.
>>>
> 
> Interesting... I do more or less that in the OpenFlow logic view,
> collapsing similar flows based on actions but not their args.
> How about showing the relevant difference in the match?
> 
> in_port(f),(src=0a:58:0a:80:02:07,dst=0a:58:0a:80:02:3b),ipv4(src=1.1.1.1,
> dst=2.2.2.2) actions=
>                                                          ipv4(
>   , dst=3.3.3.3) actions=
> 

That might be interesting.  Though the key point is that 'actions' should
be listed only once.  Because if these actions contain recirculation the
sub-trees will be identical.  They may not be identical if we filter them
out though.  But at least filtering on the input port should be safe in
that regard.

> 
>>>>
>>>> I also thought of having some interactivity (I added some very basic one
>>>> in the html format) so you could manually remove the branches you're not
>>>> interested in. But that's the only thin
>>>
>>> Yeah, I'd really like interactive filtering... maybe even not interactive for
>>> the starters.  Maybe just a string match or regex instead of packet-aware
>>> filtering.  The way it should work is to go through the tree and print out
>>> only paths that contain a match, i.e. all parents of the matching flow
>>> and children.  Basically, cut out the siblings.  For exmaple, something like
>>> --filter="in_port(18)" should only print trees that match on port 18.
>>> This is an easy one, since input port is always in the flow.
>>>
> 
> I kept the siblings to make it easy to debug what could have happened,
> e.g: after a nat. Do you see any value in keeping siblings at all
> (should I keep it as an option or just remove them altogether)?.

After NAT, they are children, not siblings, right?

> 
>>> A big problem I'm also facing is that grep doesn't work due to coloring.
>>> And if I remove the colors for grep I will not be able to add them back,
>>> so grep filtering kind of has to be supported natively in ovs-flowviz.
>>>
> 
> Why do you need to grep after ovs-floviz? e.g:
>    "grep 'input(eth0)' dpflows.txt | ovs-flowviz datapath tree"

Ah, good point.  Didn't think of that. :)

> 
> IIUC, your problem is that running "ovs-flowviz datapath console | grep |
> ovs-flowviz datapath tree" does not work because of the console's header
> and extra things, right? Those are just to make it prettier for multiple
> files, we can maybe make it optional or remove it altogether.

Yeah, I'm not sure if the header thing is useful for a cli tool.  It can
make pipeline text processing harder for sure.  For tui it's nice to have,
but not for cli.

> 
> We could have a regexp filter, but it'll probably be easier to do using
> Python's regexp syntax.

Sounds reasonable.

> 
>>> All in all, what we all want is just have retis print out datapath flows as
>>> packets match them. :D  So, we can pcap-filter a TCP stream and see all
>>> the datapath flows it matched and actions executed.
>>>
> 
> That is definitely in my todo list :-).

What's an ETA? :P

> 
>>> Best regards, Ilya Maximets.
>>>
>>>>
>>>> BTW. Thanks for trying it out and giving feedback.
>>>>
>>>> --
>>>> Adrián
>>>>
>>>
>>
>