Thank you again for all your input! Your comments have been incorporated into a new patch series v4:
https://patchwork.ozlabs.org/project/openvswitch/list/?series=382459&archive=both&state=* On 25.10.23 11:37, jm...@redhat.com wrote: > From: Jakob Meng <c...@jakobmeng.de> > > Add global option to output JSON from ovs-appctl cmds. > > This patch is an update of [0] with the following major changes: > * The JSON-RPC API change is now backward compatible. One can use an > updated client (ovs-appctl/dpctl) with an old server (ovs-vswitchd) > and vice versa. Of course, JSON output only works when both are > updated. > * tests/pmd.at from forth patch now features an example of how the > output looks like when a command does not support JSON output. > * The patch has been split into a series of four. The first patch > introduces the '-f,--format' option for ovs-appctl/ovs-dpctl and > necessary changes to the JSON-RPC API. It does not yet pass the > output format to individual commands because that requires a lot > of changes. Those changes have been split out into the third patch > to increase readability of the series. > * The second patch introduces equivalent changes to the Python files. > * The third patch moves all commands to the updated functions in > lib/unixctl.*, in particular unixctl_command_register() and the > unixctl_cb_func type, as well as their Python counterparts. The > output is still text-only (no json) for all commands. > * The forth patch shows how JSON output could be implemented using > 'dpif/show' as an example. > > The following paragraphs are taken from the previous patch revision > and have been updated to changes mentioned above. > > For monitoring systems such as Prometheus it would be beneficial if OVS > and OVS-DPDK would expose statistics in a machine-readable format. > Several approaches like UNIX socket, OVSDB queries and JSON output from > ovs-xxx tools have been proposed [2],[3]. This proof of concept > describes one way how ovs-xxx tools could output JSON in addition to > plain-text for humans. > > This patch follows an alternative approach to RFC [1] which > implemented JSON output as a separate option for each command like > 'dpif/show'. The option was called '-o|--output' in the latter. It > has been renamed to '-f,--format' because ovs-appctl already has a > short option '-o' which prints the available ovs-appctl options > ('--option'). The new option name '-f,--format' is in line with > ovsdb-client where it controls output formatting, too. > > An example call would be 'ovs-appctl --format json dpif/show' as > shown in tests/pmd.at of the forth patch. By default, the output > format is plain-text as before. > > With this patch, all commands announce their support for output > formats when being registered with unixctl_command_register() from > lib/unixctl.*, e.g. OVS_OUTPUT_FMT_TEXT and/or OVS_OUTPUT_FMT_JSON. > When a requested output format is not supported by a command, then > process_command() in lib/unixctl.c will return an error. This is an > advantage over the previous approach [1] where each command would have > to parse the output format option and handle requests for unsupported > output formats on its own. > > The question whether JSON output should be pretty printed and sorted > remains. In most cases it would be unnecessary because machines > consume the output or humans could use jq for pretty printing. However, > it would make tests more readable (for humans) without having to use jq > (which would require us to introduce a dependency on jq). > > Wdyt? > > [0] > https://patchwork.ozlabs.org/project/openvswitch/patch/20231020104320.1417664-2-jm...@redhat.com/ > [1] > https://patchwork.ozlabs.org/project/openvswitch/patch/20231020092205.710399-2-jm...@redhat.com/ > [2] https://bugzilla.redhat.com/show_bug.cgi?id=1824861#c7 > [3] > https://patchwork.ozlabs.org/project/openvswitch/patch/20230831131122.284913-1-rja...@redhat.com/ > > Reported-at: https://bugzilla.redhat.com/1824861 > Signed-off-by: Jakob Meng <c...@jakobmeng.de> > > Jakob Meng (4): > Add global option for JSON output to ovs-appctl/dpctl. > python: Add global option for JSON output to Python tools. > Migrate commands to extended unixctl API. > ofproto: Add JSON output for 'dpif/show' command. > > lib/bfd.c | 16 ++- > lib/cfm.c | 13 +- > lib/command-line.c | 36 +++++ > lib/command-line.h | 10 ++ > lib/coverage.c | 11 +- > lib/dpctl.c | 129 ++++++++++------- > lib/dpctl.h | 4 + > lib/dpdk.c | 15 +- > lib/dpif-netdev-perf.c | 1 + > lib/dpif-netdev-perf.h | 1 + > lib/dpif-netdev.c | 84 ++++++----- > lib/dpif-netlink.c | 6 +- > lib/lacp.c | 7 +- > lib/memory.c | 5 +- > lib/netdev-dpdk.c | 16 ++- > lib/netdev-dummy.c | 30 ++-- > lib/netdev-native-tnl.c | 4 +- > lib/netdev-native-tnl.h | 4 +- > lib/netdev-vport.c | 1 + > lib/odp-execute.c | 10 +- > lib/ovs-lldp.c | 16 ++- > lib/ovs-router.c | 26 ++-- > lib/rstp.c | 22 +-- > lib/stopwatch.c | 10 +- > lib/stp.c | 20 +-- > lib/timeval.c | 13 +- > lib/tnl-neigh-cache.c | 35 +++-- > lib/tnl-ports.c | 6 +- > lib/unixctl.c | 246 ++++++++++++++++++++++++--------- > lib/unixctl.h | 11 +- > lib/vlog.c | 46 ++++-- > ofproto/bond.c | 32 +++-- > ofproto/ofproto-dpif-trace.c | 10 +- > ofproto/ofproto-dpif-upcall.c | 81 ++++++++--- > ofproto/ofproto-dpif.c | 187 +++++++++++++++++++++---- > ofproto/ofproto.c | 10 +- > ofproto/tunnel.c | 4 +- > ovsdb/file.c | 4 +- > ovsdb/ovsdb-client.c | 40 ++++-- > ovsdb/ovsdb-server.c | 122 +++++++++++----- > ovsdb/ovsdb.c | 4 +- > ovsdb/raft.c | 28 ++-- > python/ovs/unixctl/__init__.py | 19 ++- > python/ovs/unixctl/client.py | 20 ++- > python/ovs/unixctl/server.py | 64 +++++---- > python/ovs/util.py | 9 ++ > python/ovs/vlog.py | 12 +- > tests/appctl.py | 7 +- > tests/pmd.at | 34 +++++ > tests/test-netflow.c | 4 +- > tests/test-sflow.c | 4 +- > tests/test-unixctl.c | 33 +++-- > tests/test-unixctl.py | 25 ++-- > utilities/ovs-appctl.c | 65 +++++++-- > utilities/ovs-dpctl.c | 12 ++ > utilities/ovs-ofctl.c | 43 +++--- > vswitchd/bridge.c | 25 ++-- > vswitchd/ovs-vswitchd.c | 5 +- > 58 files changed, 1266 insertions(+), 491 deletions(-) > > -- > 2.39.2 > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
