On 5/16/24 17:41, [email protected] wrote:
> From: Jakob Meng <[email protected]>
>
> The 'dpif/show' command now supports machine-readable JSON output in
> addition to the plain-text output for humans. An example would be:
>
> ovs-appctl --format json dpif/show
>
> Reported-at: https://bugzilla.redhat.com/1824861
> Signed-off-by: Jakob Meng <[email protected]>
> ---
> NEWS | 1 +
> ofproto/ofproto-dpif.c | 120 +++++++++++++++++++++++++++++++++++++----
> tests/pmd.at | 23 ++++++++
> 3 files changed, 133 insertions(+), 11 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 479310d49..c434b1497 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,7 @@ Post-v3.3.0
> or 'text' (by default).
> * Added new option [--pretty] to print JSON output in a readable
> fashion.
> * 'list-commands' now supports output in JSON format.
> + * 'dpif/show' now supports output in JSON format.
Might be better to squash above two entries into one.
> - Python:
> * Added support for different output formats like 'json' to appctl.py
> and
> Python's unixctl classes.
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 32d037be6..db0405886 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -28,6 +28,7 @@
> #include "fail-open.h"
> #include "guarded-list.h"
> #include "hmapx.h"
> +#include "json.h"
> #include "lacp.h"
> #include "learn.h"
> #include "mac-learning.h"
> @@ -6519,8 +6520,99 @@ done:
> return changed;
> }
>
> +static struct json *
> +dpif_show_backer_json(struct json *backers, const struct dpif_backer *backer)
> +{
> + struct json *json_backer = json_object_create();
> +
> + /* Add datapath as new JSON object using its name as key. */
> + json_object_put(backers, dpif_name(backer->dpif), json_backer);
> +
> + /* Add datapath's stats under "stats" key. */
> + struct dpif_dp_stats dp_stats;
> + struct json *json_dp_stats = json_object_create();
Reverse x-mass tree.
> +
> + json_object_put(json_backer, "stats", json_dp_stats);
> + dpif_get_dp_stats(backer->dpif, &dp_stats);
> + json_object_put_format(json_dp_stats, "n_hit", "%"PRIu64,
> dp_stats.n_hit);
> + json_object_put_format(json_dp_stats, "n_missed", "%"PRIu64,
> + dp_stats.n_missed);
It's better to build the json_dp_stats first, and then add it to json_backer.
> +
> + /* Add datapath's bridges under "bridges" key. */
> + struct json *json_dp_bridges = json_object_create();
> + json_object_put(json_backer, "bridges", json_dp_bridges);
Move to the end.
> +
> + struct shash ofproto_shash;
> + shash_init(&ofproto_shash);
Use SHASH_INITIALIZER.
> + const struct shash_node **ofprotos = get_ofprotos(&ofproto_shash);
> + for (size_t i = 0; i < shash_count(&ofproto_shash); i++) {
But also, there is no need to sort things while adding to json objects.
The order will be lost and we'll re-sort on output.
So, we could just free(get_ofprotos(&ofproto_shash)); and then use normal
SHASH_FOR_EACH to iterate.
> + struct ofproto_dpif *ofproto = ofprotos[i]->data;
> +
> + if (ofproto->backer != backer) {
> + continue;
> + }
> +
> + struct json *json_ofproto = json_object_create();
> + /* Add bridge to "bridges" dictionary using its name as key. */
> + json_object_put(json_dp_bridges, ofproto->up.name, json_ofproto);
Move to the end.
> +
> + /* Add bridge ports to the current bridge dictionary. */
> + const struct shash_node **ports;
> + ports = shash_sort(&ofproto->up.port_by_name);
> + for (size_t j = 0; j < shash_count(&ofproto->up.port_by_name); j++) {
Just iterate SHASH directly.
> + const struct shash_node *port = ports[j];
> + struct ofport *ofport = port->data;
> +
> + struct json * json_ofproto_port = json_object_create();
struct json *json_ofproto_port
> + /* Add bridge port to a bridge's dict using port name as key. */
> + json_object_put(json_ofproto, netdev_get_name(ofport->netdev),
> + json_ofproto_port);
To the end.
> + /* Add OpenFlow port associated with a bridge port. */
> + json_object_put_format(json_ofproto_port, "ofport", "%u",
PRIu32
> + ofport->ofp_port);
> +
> + /* Add bridge port number. */
> + odp_port_t odp_port = ofp_port_to_odp_port(ofproto,
> + ofport->ofp_port);
> + if (odp_port != ODPP_NONE) {
> + json_object_put_format(json_ofproto_port, "port_no",
> + "%"PRIu32, odp_port);
> + } else {
> + json_object_put_string(json_ofproto_port, "port_no", "none");
> + }
> +
> + /* Add type of a bridge port. */
> + json_object_put_string(json_ofproto_port, "type",
> + netdev_get_type(ofport->netdev));
> +
> + /* Add config entries for a bridge port. */
> + struct json *json_port_config = json_object_create();
> + struct smap config;
> + smap_init(&config);
SMAP_INITIALIZER
> +
> + json_object_put(json_ofproto_port, "config", json_port_config);
To the end.
> + if (!netdev_get_config(ofport->netdev, &config)) {
> + struct smap_node *node;
> +
> + SMAP_FOR_EACH (node, &config) {
> + json_object_put_string(json_port_config, node->key,
> + node->value);
> + }
> + }
> + smap_destroy(&config);
> +
> + } /* End of bridge port(s). */
> +
> + free(ports);
> + } /* End of bridge(s). */
An empty line would be nice.
> + shash_destroy(&ofproto_shash);
> + free(ofprotos);
> +
> + return json_backer;
> +}
> +
> static void
> -dpif_show_backer(const struct dpif_backer *backer, struct ds *ds)
> +dpif_show_backer_text(const struct dpif_backer *backer, struct ds *ds)
> {
> const struct shash_node **ofprotos;
> struct dpif_dp_stats dp_stats;
> @@ -6587,18 +6679,24 @@ static void
> ofproto_unixctl_dpif_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> const char *argv[] OVS_UNUSED, void *aux
> OVS_UNUSED)
> {
> - struct ds ds = DS_EMPTY_INITIALIZER;
> - const struct shash_node **backers;
> - int i;
> + if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> + struct json *backers = json_object_create();
> + const struct shash_node *backer;
An empty line.
> + SHASH_FOR_EACH (backer, &all_dpif_backers) {
> + dpif_show_backer_json(backers, backer->data);
> + }
> + unixctl_command_reply_json(conn, backers);
> + } else {
> + struct ds ds = DS_EMPTY_INITIALIZER;
> + const struct shash_node **backers = shash_sort(&all_dpif_backers);
An empty line. Also, reverse x-mass tree.
> + for (int i = 0; i < shash_count(&all_dpif_backers); i++) {
> + dpif_show_backer_text(backers[i]->data, &ds);
> + }
> + free(backers);
>
> - backers = shash_sort(&all_dpif_backers);
> - for (i = 0; i < shash_count(&all_dpif_backers); i++) {
> - dpif_show_backer(backers[i]->data, &ds);
> + unixctl_command_reply(conn, ds_cstr(&ds));
> + ds_destroy(&ds);
> }
> - free(backers);
> -
> - unixctl_command_reply(conn, ds_cstr(&ds));
> - ds_destroy(&ds);
> }
>
> static void
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 35a44b4df..189138f81 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
pmd.at is not a right place for a test. There should be a test
in ofproto-dpif.at instead. We already have one there for this
particular appctl command.
> @@ -112,6 +112,29 @@ dummy@ovs-dummy: hit:0 missed:0
> p0 1/1: (dummy-pmd: n_rxq=1, n_txq=1, numa_id=0)
> ])
>
> +AT_CHECK([ovs-appctl --format json --pretty dpif/show], [0], [dnl
> +[{
> + "dummy@ovs-dummy": {
> + "bridges": {
> + "br0": {
> + "br0": {
> + "config": {
> + },
> + "ofport": "65534",
> + "port_no": "100",
> + "type": "dummy-internal"},
> + "p0": {
> + "config": {
> + "n_rxq": "1",
> + "n_txq": "1",
> + "numa_id": "0"},
> + "ofport": "1",
> + "port_no": "1",
> + "type": "dummy-pmd"}}},
> + "stats": {
> + "n_hit": "0",
> + "n_missed": "0"}}}]])
Normal command doesn't have 'n_' prefixes.
> +
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
