On 25 Oct 2023, at 11:41, Jakob Meng wrote:
> On 20.10.23 13:48, Eelco Chaudron wrote: >> >> On 20 Oct 2023, at 12:43, [email protected] wrote: >> >>> From: Jakob Meng <[email protected]> >>> >>> This patch follows an alternative approach to RFC [0]. >>> >>> 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 [1],[2]. This proof of concept >>> describes one way how ovs-xxx tools could output JSON in addition to >>> plain-text for humans. >>> >>> In the previous approach JSON output was implemented as a separate >>> option for each command such as 'dpif/show'. This patch adds a global >>> option '-f,--format' instead. An example call would be >>> 'ovs-appctl --format json dpif/show' as shown in tests/pmd.at. >>> By default, the output format is plain-text as before. >>> >>> In the previous patch the option was called '-o|--output' instead of >>> '-f,--format'. But 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. >>> >>> unixctl_command_register() in lib/unixctl.* gained a new int arg >>> 'output_fmts' with which commands have to announce their support for >>> output formats. All commands have been updated accordingly which is why >>> so many files had to be changed. Most announce OVS_OUTPUT_FMT_TEXT only >>> except for 'dpif/show' in ofproto/ofproto-dpif.c which also supports >>> OVS_OUTPUT_FMT_JSON. >>> >>> The JSON-RPC API in lib/unixctl.c has been changed to transport the >>> requested output format besides command and args. Previously, the >>> command was copied to the 'method' parameter in JSON-RPC while the >>> args were copied to the 'params' parameter. Without any other means >>> to transport parameters via JSON-RPC left, the meaning of 'method' >>> and 'params' had to be changed: 'method' will now always be >>> 'execute/v1' to ensure (in)compatibility between (mis)matching client >>> and server. The JSON-RPC 1.0 spec which is currently implemented, >>> defines 'params' as a JSON array. The first parameter will now be the >>> command, the second one the output format and the rest will be command >>> args. >>> >>> Function type unixctl_cb_func in lib/unixctl.h has gained a new arg >>> 'enum ovs_output_fmt fmt'. The output format which is passed via unix >>> socket to ovs-vswitchd will be converted into a value of type >>> 'enum ovs_output_fmt' in lib/unixctl.c and then passed to said 'fmt' >>> arg of the choosen command handler. 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 [0] where each command >>> would have to parse the output format option and handle requests for >>> unsupported output formats on its own. >>> >>> A disadvantage to the previous approach is that the JSON-RPC API for >>> the socket communication had to be changed and all commands had to be >>> updated to announce their output format support. However, this is a >>> one-time change only: Whenever JSON output is to be implemend for >>> another command, OVS_OUTPUT_FMT_TEXT|OVS_OUTPUT_FMT_JSON would have to >>> be added to the unixctl_command_register() call and the command handler >>> would have to readout the fmt arg. >>> >>> 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? >> See my comments on the v1, which crossed: >> >> https://mail.openvswitch.org/pipermail/ovs-dev/2023-October/408810.html >> > > Thank you, Eelco ☺️ > > I have incorporated all your comments into a new patch series: > > https://patchwork.ozlabs.org/project/openvswitch/list/?series=379267&state=* > > Cover letter explains it all but is not shown in patchwork... Thanks, will at it to my review list. //Eelco _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
