On 1/8/2020 5:56 PM, Ilya Maximets wrote:
On 08.01.2020 18:37, Stokes, Ian wrote:
On 1/8/2020 4:38 PM, Ilya Maximets wrote:
On 07.01.2020 11:51, Stokes, Ian wrote:
On 1/2/2020 12:27 PM, Ilya Maximets wrote:
On 20.12.2019 16:28, Emma Finn wrote:
Add an ovs-appctl command to iterate through the dpcls
and for each subtable output the miniflow bits for any
existing table.
$ ovs-appctl dpif-netdev/subtable-show
pmd thread numa_id 0
dpcls port 2:
subtable:
unit_0: 2 (0x5)
unit_1: 1 (0x1)
pmd thread numa_id 1
dpcls port 3:
subtable:
unit_0: 2 (0x5)
unit_1: 1 (0x1)
Signed-off-by: Emma Finn <[email protected]>
---
So, what about my suggestions and thoughts about alternative solutions
that I posted in reply to RFC? It's still unclear why we need to disturb
the running datapath to get this information, especially if we could get
it "offline" from the flow dump.
Hi Ilya,
apologies I've only spotted this post now.
I guess to my mind there are a few reasons why this is being added as a new
command and not a separate application to process/parse the flow dumps of a
datapath.
(i) Ease of implementation, it seems straight forward enough to follow the
existing commands structure such as dpif-netdev/pmd-rxq-show to implement this
command. It had the required elements (required data structures etc.) so
minimum plumbing was required to get access to that info and it will be
familiar to any other developers who have already worked or will work in that
area of the code in the future.
I agree this could be done offline without the need to lock the datapath, but
from my understanding I don't think the intention here is to run this command
at a frequent or high interval so I would think that the lock should not be an
issue unless the command is being executed continually (again similar to
pmd-rxq-show, it would be called when needed only).
The concern of adding a new separate application for parsing the dump flow as
you suggested came down to it being another separate app within OVS to maintain
as well as the work required to plumb or parse all required info.
It's not hard to parse. Just a few function calls. Most of the required
functionality exists in 'lib' code.
If you don't like a separate app or script, this information could be printed
I think it would be better to keep it in OVS rather than a separate app/script.
in a flow-dump in a some special field named like 'dp-extra-info' or whatever.
Like this:
recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
dp-extra-info:miniflow_bits(0x5, 0x1), actions:hash(sym_l4(0)),recirc(0x1)
This is interesting approach, but are we missing info such as the in ports,
numa nodes etc? Maybe Harry can comment to this as it was his idea originally.
It seems with either approach you will be missing some info depending on how
you want to approach the debug usecase.
We're not missing core info since flow dumps are printed per-PMD:
flow-dump from the main thread:
<...>
flow-dump from the pmd thread on core X:
<...>
numa could be easily checked by by the core number and I'm not sure
if we really need this information here.
Flow dumps always contains 'in_port' fileld and the port name will
be printed instead of port number if you'll pass --names to the dump command.
'dp-extra-info' might be printed with verbosity enabled if dpif provided this
information.
I haven't looked at ovs-ofctl myself, not sure how much work would be needed to
access this info or if you are back to parsing, again I think the idea here is
we have this info available in the current approach as is why parse and break
out these values?
Not ovs-ofctl - It dumps OF tables, not the datapath flow tables - but
'ovs-dpctl dump-flows' or 'ovs-appctl dpctl/dump-flows'. dpctl and appctl
are using same code from lib/dpctl.c which is fairly simple. I'm suggesting
to add this information to dpif_flow.attrs and dpif-netdev will fill this
information while sending dumped flow in dp_netdev_flow_to_dpif_flow().
Should not be hard.
It's not parsing and breaking out, we have a netdev_flow while dumping
and we only need to store a bit more information from it into dpif_flow.
Now I'm on the same page as you, apologies, previously thought you were
referring to ofctl. This approach seems more appropriate.
Would it even be acceptable for 2.13 as well? it would be a new v1 and we're
past the soft freeze.
Technically, soft freeze wasn't announced yet. =)
And we also could make it a v3.
+1 :D
I really feel that this information (miniflow bits) should be bonded with
flow-dump
somehow just because it belongs there.
I guess we're looking at this from different points of view, don't shoot me but
would it make sense to have both approaches? :-D
Not sure about that. I'd like to not have both.
No worries, I understand what you meant better now so agree it will be
fine in one place.
Best Regards
Ian
After posting the RFC we had a number of users already applying the patch and
using it in their deployments, we spoke about it at the conference and didn't
hear any objections so I this is why the patch has continued with this approach
for the 2.13 release.
They didn't have any alternatives to use. =)
Agree, but the choice provided worked well so it may not have been a problem :D.
Best Regards
Ian
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
