From patchwork Thu Nov 16 10:41:20 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakob Meng X-Patchwork-Id: 1864684 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=VryxRaTF; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4SWGnB69wsz1yRR for ; Thu, 16 Nov 2023 21:42:06 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 1133F426CA; Thu, 16 Nov 2023 10:42:04 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 1133F426CA Authentication-Results: smtp4.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=VryxRaTF X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0cS85oHQNzGY; Thu, 16 Nov 2023 10:42:00 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id 42AAD42527; Thu, 16 Nov 2023 10:41:58 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 42AAD42527 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 84E1CC008C; Thu, 16 Nov 2023 10:41:54 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id DB04EC0DD7 for ; Thu, 16 Nov 2023 10:41:52 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 510B683294 for ; Thu, 16 Nov 2023 10:41:52 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 510B683294 Authentication-Results: smtp1.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=VryxRaTF X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dA02LduMWZor for ; Thu, 16 Nov 2023 10:41:50 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 4785E8322A for ; Thu, 16 Nov 2023 10:41:50 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 4785E8322A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700131309; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=BJeKw6RVJYM638q2HUr9qlq/y4BDgRvaff0cKkQvBWM=; b=VryxRaTFaTmsMwTDl32OQtRDxbTZ9qT82eT1K34Vm1uM1Q+Ay0GB8z1lsiXzReF905B0r6 Avs0PKvClYewwGI4f5mPwshvlqYMHdz5iDGDJ6daS9gIdcGntxBsA17/x7NoWJW6EwEFyv vEtOdqRWCwx2x8qr8ufmsT5CuFV3FGM= Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-75-mXzhBF6zONGx88UH-PVMcQ-1; Thu, 16 Nov 2023 05:41:47 -0500 X-MC-Unique: mXzhBF6zONGx88UH-PVMcQ-1 Received: by mail-lj1-f198.google.com with SMTP id 38308e7fff4ca-2c6f33ee403so5581191fa.2 for ; Thu, 16 Nov 2023 02:41:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700131306; x=1700736106; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BJeKw6RVJYM638q2HUr9qlq/y4BDgRvaff0cKkQvBWM=; b=LSK4t70T861gjWLo3uSq9JvFRMlNPC6Mp+RPEkzjrS+3qXvXCGOzaACTw6YOjhrW/c sU42EaMP9lkMaqDy0bKkmdSdzL7ZSAqfkemAJD5CuJiw6YYdfW25Ez7I40cMGF2Ztam6 iOwJ4l+IoTUKy3kh2x3bBb0OB7NKrYp44CcqazxVO6aH6WMICsh4BW2uu6B2An31UjQx hTOA0qJk4O/cZOHPwqisnItSskms0ss8DrG2L7dd+LLXKWTC1GuvZC8D0+czYXe6ZxKL Z2f0JdF0XoTK/l0slzlIQuqoL0OgK0BUwXXvlzRzGSFyDAoV+DK+R8gLBDFomnq79v1t +qRg== X-Gm-Message-State: AOJu0YyQK7Ssl4c8t8zmdcbMvuZuAY5k7WBxTPTM+ULgCqb8GQ/bPLWm ZB9Z6fGmZTIz0tFoWq1RqAFn7e20oy+TFIXksZBbqH1ghbvt2tsT41z9GXi1iJNuHB7GzKepfIP ZgFhybc/mYvt3BRC3dcmqXxbB9JO+hjw+EkdrCrQS9VW5x+WiTV4D737Bpvgauuqxucw= X-Received: by 2002:a2e:a4cb:0:b0:2c5:5926:de52 with SMTP id p11-20020a2ea4cb000000b002c55926de52mr5061894ljm.53.1700131305939; Thu, 16 Nov 2023 02:41:45 -0800 (PST) X-Google-Smtp-Source: AGHT+IFsKh0+NnsxJhcsiy6aPlj78Q6m5PbGDPCnzc02+E3zdD0XUgC6gu96ssAxgxpWcrY3iN4GAg== X-Received: by 2002:a2e:a4cb:0:b0:2c5:5926:de52 with SMTP id p11-20020a2ea4cb000000b002c55926de52mr5061871ljm.53.1700131305489; Thu, 16 Nov 2023 02:41:45 -0800 (PST) Received: from positronik4lide.redhat.com ([87.122.56.153]) by smtp.gmail.com with ESMTPSA id n10-20020a05600c304a00b004080f0376a0sm2937210wmh.42.2023.11.16.02.41.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 16 Nov 2023 02:41:45 -0800 (PST) From: jmeng@redhat.com To: dev@openvswitch.org, i.maximets@ovn.org, echaudro@redhat.com, ktraynor@redhat.com, aconole@redhat.com, rjarry@redhat.com Date: Thu, 16 Nov 2023 11:41:20 +0100 Message-Id: <20231116104120.25676-7-jmeng@redhat.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231116104120.25676-1-jmeng@redhat.com> References: <20231116104120.25676-1-jmeng@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v4 6/6] unixctl: Pass output format as command args via JSON-RPC API. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Jakob Meng A previous patch had changed the JSON-RPC API in lib/unixctl.* (and its Python counterpart) in order to allow transporting the requested output format from ovs-xxx tools to ovs-vswitchd across unix sockets: The meaning of 'method' and 'params' had be changed: 'method' would be 'execute/v1' when an output format other than 'text' is requested. In that case, the first parameter of the JSON array 'params' would be the designated command, the second one the output format and the rest will be command args. That change allowed to transport the output format in a backward compatible way: One could use an updated client (ovs-appctl) with an old server (ovs-vswitchd) and vice versa. Of course, JSON output would only work when both sides have been updated. This patch reverts the JSON-RPC API to the old behaviour: The command will be copied to JSON-RPC's 'method' parameter while command args will be copied to the 'params' parameter. The client will inject the output format into the command args and the server will parse and remove it before passing the args to the command callback. The advantage of this (old) approach is a simpler JSON-RPC API but at the cost of inferior usability: ovs-xxx tools are no longer able to detect missing JSON support at the server side. Old servers will simply pass the output format as an arg to the command which would then result in an error such as 'command takes at most ... arguments' or a non-\ uniform error from the command callback. Signed-off-by: Jakob Meng --- lib/unixctl.c | 147 ++++++++++++++++------------------- python/ovs/unixctl/client.py | 19 ++--- python/ovs/unixctl/server.py | 37 ++++----- python/ovs/util.py | 1 - 4 files changed, 91 insertions(+), 113 deletions(-) diff --git a/lib/unixctl.c b/lib/unixctl.c index e7edbb154..1eaf6edb0 100644 --- a/lib/unixctl.c +++ b/lib/unixctl.c @@ -63,8 +63,6 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); static struct shash commands = SHASH_INITIALIZER(&commands); -static const char *rpc_marker = "execute/v1"; - static void unixctl_list_commands(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[] OVS_UNUSED, @@ -292,11 +290,9 @@ process_command(struct unixctl_conn *conn, struct jsonrpc_msg *request) struct unixctl_command *command; struct json_array *params; - const char *method; - enum ovs_output_fmt fmt; + enum ovs_output_fmt fmt = OVS_OUTPUT_FMT_TEXT; struct svec argv = SVEC_EMPTY_INITIALIZER; - int args_offset; - bool plain_rpc; + int argc; COVERAGE_INC(unixctl_received); conn->request_id = json_clone(request->id); @@ -310,20 +306,9 @@ process_command(struct unixctl_conn *conn, struct jsonrpc_msg *request) free(id_s); } - /* The JSON-RPC API requires an indirection in order to allow transporting - * additional data like the output format besides command and args. For - * backward compatibility with older clients the plain RPC is still - * supported. */ - plain_rpc = strcmp(request->method, rpc_marker); - args_offset = plain_rpc ? 0 : 2; - params = json_array(request->params); - if (!plain_rpc && (params->n < 2)) { - error = xasprintf("JSON-RPC API mismatch: Unexpected # of params:"\ - " %"PRIuSIZE, params->n); - goto error; - } + /* Verify type of arguments. */ for (size_t i = 0; i < params->n; i++) { if (params->elems[i]->type != JSON_STRING) { error = xasprintf("command has non-string argument: %s", @@ -332,54 +317,70 @@ process_command(struct unixctl_conn *conn, struct jsonrpc_msg *request) } } - /* Extract method name. */ - method = plain_rpc ? request->method : json_string(params->elems[0]); + argc = params->n; - /* Extract output format. */ - if (plain_rpc) { - fmt = OVS_OUTPUT_FMT_TEXT; - } else { - if (!ovs_output_fmt_from_string(json_string(params->elems[1]), &fmt)) { - error = xasprintf("invalid output format: %s", - json_string(params->elems[1])); - goto error; + /* Extract and process command args. */ + svec_add(&argv, request->method); + for (size_t i = 0; i < params->n; i++) { + if (!strcmp(json_string(params->elems[i]), "-f") || + !strcmp(json_string(params->elems[i]), "--format")) + { + /* Parse output format argument. */ + + if ((i + 1) == (params->n)) { + error = xasprintf("option requires an argument -- %s", + json_string(params->elems[i])); + goto error; + } + + /* Move index to option argument. */ + i++; + + if (!ovs_output_fmt_from_string(json_string(params->elems[i]), + &fmt)) + { + error = xasprintf("option %s has invalid value %s", + json_string(params->elems[i - 1]), + json_string(params->elems[i])); + goto error; + } + + argc -= 2; + } else { + /* Pass other arguments to command. */ + svec_add(&argv, json_string(params->elems[i])); } } + svec_terminate(&argv); /* Find command with method name. */ - command = shash_find_data(&commands, method); + command = shash_find_data(&commands, request->method); /* Verify that method call complies with command requirements. */ if (!command) { error = xasprintf("\"%s\" is not a valid command (use " "\"list-commands\" to see a list of valid commands)", - method); + request->method); goto error; - } else if ((params->n - args_offset) < command->min_args) { + } else if (argc < command->min_args) { error = xasprintf("\"%s\" command requires at least %d arguments", - method, command->min_args); + request->method, command->min_args); goto error; - } else if ((params->n - args_offset) > command->max_args) { + } else if (argc > command->max_args) { error = xasprintf("\"%s\" command takes at most %d arguments", - method, command->max_args); + request->method, command->max_args); goto error; } else if ((!command->output_fmts && fmt != OVS_OUTPUT_FMT_TEXT) || (command->output_fmts && !(fmt & command->output_fmts))) { error = xasprintf("\"%s\" command does not support output format"\ - " \"%s\" %d %d", method, - ovs_output_fmt_to_string(fmt), command->output_fmts, + " \"%s\" %d %d", request->method, + ovs_output_fmt_to_string(fmt), + command->output_fmts, fmt); goto error; } - /* Extract command args. */ - svec_add(&argv, method); - for (size_t i = args_offset; i < params->n; i++) { - svec_add(&argv, json_string(params->elems[i])); - } - svec_terminate(&argv); - command->cb(conn, argv.n, (const char **) argv.names, fmt, command->aux); svec_destroy(&argv); @@ -387,6 +388,9 @@ process_command(struct unixctl_conn *conn, struct jsonrpc_msg *request) return; error: unixctl_command_reply_error(conn, error); + if (!svec_is_empty(&argv)) { + svec_destroy(&argv); + } free(error); } @@ -558,39 +562,32 @@ unixctl_client_transact(struct jsonrpc *client, const char *command, int argc, { struct jsonrpc_msg *request, *reply; struct json **json_args, *params; - int error, i; - /* The JSON-RPC API requires an indirection in order to allow transporting - * additional data like the output format besides command and args. For - * backward compatibility with older servers the plain RPC is still - * supported. */ - bool plain_rpc = (fmt == OVS_OUTPUT_FMT_TEXT); + int error, i, argcx = 0; + + if (fmt != OVS_OUTPUT_FMT_TEXT) { + argcx += 2; + } *result = NULL; *err = NULL; - if (plain_rpc) { - json_args = xmalloc(argc * sizeof *json_args); - for (i = 0; i < argc; i++) { - json_args[i] = json_string_create(argv[i]); - } - - params = json_array_create(json_args, argc); - request = jsonrpc_create_request(command, params, NULL); - } else { - json_args = xmalloc((argc + 2) * sizeof *json_args); - json_args[0] = json_string_create(command); - json_args[1] = ovs_output_fmt_to_json(fmt); - for (i = 0; i < argc; i++) { - json_args[i + 2] = json_string_create(argv[i]); - } - - params = json_array_create(json_args, argc + 2); + json_args = xmalloc((argc + argcx) * sizeof *json_args); + for (i = 0; i < argc; i++) { + json_args[i] = json_string_create(argv[i]); + } - /* Use a versioned command to ensure that both server and client - * use the same JSON-RPC API. */ - request = jsonrpc_create_request(rpc_marker, params, NULL); + /* Pass output format for non-text only to keep compatibility with old + servers. */ + if (fmt != OVS_OUTPUT_FMT_TEXT) { + json_args[i] = json_string_create("--format"); + ++i; + json_args[i] = ovs_output_fmt_to_json(fmt); + ++i; } + params = json_array_create(json_args, argc + argcx); + request = jsonrpc_create_request(command, params, NULL); + error = jsonrpc_transact_block(client, request, &reply); if (error) { VLOG_WARN("error communicating with %s: %s", jsonrpc_get_name(client), @@ -600,17 +597,7 @@ unixctl_client_transact(struct jsonrpc *client, const char *command, int argc, if (reply->error) { if (reply->error->type == JSON_STRING) { - /* Catch incompatible server and return helpful error msg. */ - char *plain_rpc_error = xasprintf("\"%s\" is not a valid command", - rpc_marker); - if (!strncmp(plain_rpc_error, json_string(reply->error), - strlen(plain_rpc_error))) { - *err = xstrdup("JSON RPC reply indicates incompatible server. " - "Please upgrade server side to newer version."); - } else { - *err = xstrdup(json_string(reply->error)); - } - free(plain_rpc_error); + *err = xstrdup(json_string(reply->error)); } else { VLOG_WARN("%s: unexpected error type in JSON RPC reply: %s", jsonrpc_get_name(client), diff --git a/python/ovs/unixctl/client.py b/python/ovs/unixctl/client.py index 29a5f3df9..bd690d099 100644 --- a/python/ovs/unixctl/client.py +++ b/python/ovs/unixctl/client.py @@ -32,13 +32,11 @@ class UnixctlClient(object): for arg in argv: assert isinstance(arg, str) assert isinstance(fmt, ovs.util.OutputFormat) - plain_rpc = (fmt == ovs.util.OutputFormat.TEXT) - if plain_rpc: - request = ovs.jsonrpc.Message.create_request(command, argv) - else: - request = ovs.jsonrpc.Message.create_request( - "execute/v1", [command, fmt.name.lower()] + argv) + request = ovs.jsonrpc.Message.create_request( + command, + argv + ([] if (fmt == ovs.util.OutputFormat.TEXT) + else ["--format", fmt.name.lower()])) error, reply = self._conn.transact_block(request) @@ -48,14 +46,7 @@ class UnixctlClient(object): return error, None, None if reply.error is not None: - plain_rpc_error = ('"%s" is not a valid command' % - ovs.util.RPC_MARKER) - if str(reply.error).startswith(plain_rpc_error): - return 0, "JSON RPC reply indicates incompatible "\ - "server. Please upgrade server side to "\ - "newer version.", None - else: - return 0, str(reply.error), None + return 0, str(reply.error), None else: assert reply.result is not None return 0, None, str(reply.result) diff --git a/python/ovs/unixctl/server.py b/python/ovs/unixctl/server.py index 849739e89..2db855948 100644 --- a/python/ovs/unixctl/server.py +++ b/python/ovs/unixctl/server.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import argparse import copy import errno import os @@ -105,34 +106,32 @@ class UnixctlConnection(object): self._request_id = request.id try: - plain_rpc = request.method != ovs.util.RPC_MARKER - args_offset = 0 if plain_rpc else 2 - - if not plain_rpc and len(request.params) < 2: - raise ValueError( - "JSON-RPC API mismatch: Unexpected # of params: %d" - % len(request.params)) - for param in request.params: if not isinstance(param, str): raise ValueError( "command has non-string argument: %s" % param) - method = request.method if plain_rpc else request.params[0] - if plain_rpc: - fmt = ovs.util.OutputFormat.TEXT - else: - fmt = ovs.util.OutputFormat[request.params[1].upper()] - params = request.params[args_offset:] + params = request.params + + parser = argparse.ArgumentParser() + parser.add_argument("argv", nargs="*") + parser.add_argument("-f", "--format", + default="text", + choices=[fmt.name.lower() + for fmt in ovs.util.OutputFormat]) + args = parser.parse_args(params) + fmt = ovs.util.OutputFormat[args.format.upper()] + method = request.method command = ovs.unixctl.commands.get(method) + if command is None: raise ValueError('"%s" is not a valid command' % method) - elif len(params) < command.min_args: + elif len(args.argv) < command.min_args: raise ValueError( '"%s" command requires at least %d arguments' % (method, command.min_args)) - elif len(params) > command.max_args: + elif len(args.argv) > command.max_args: raise ValueError( '"%s" command takes at most %d arguments' % (method, command.max_args)) @@ -141,8 +140,10 @@ class UnixctlConnection(object): ' "%s" %d %d' % (method, fmt.name.lower(), command.output_fmts, fmt)) - unicode_params = [str(p) for p in params] - command.callback(self, unicode_params, fmt, command.aux) + command.callback(self, + args.argv, + fmt, + command.aux) except Exception as e: self.reply_error(str(e)) diff --git a/python/ovs/util.py b/python/ovs/util.py index 75ba4ee12..272ca683d 100644 --- a/python/ovs/util.py +++ b/python/ovs/util.py @@ -19,7 +19,6 @@ import enum PROGRAM_NAME = os.path.basename(sys.argv[0]) EOF = -1 -RPC_MARKER = "execute/v1" @enum.unique