Please, keep the list in CC. :)

On 5/22/24 12:52, amore...@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, amore...@redhat.com wrote:
>>>> On Tue, May 21, 2024 at 08:03:49PM GMT, Ilya Maximets wrote:
>>>>> On 5/21/24 18:13, amore...@redhat.com wrote:
>>>>>> On Tue, May 21, 2024 at 03:49:03PM GMT, amore...@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
>>>>
>>>
>>
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to