| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YtpCCFiXSAFWoUdg@nanopsycho>
Date: Fri, 22 Jul 2022 08:22:00 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: netdev@...r.kernel.org, Jonathan Corbet <corbet@....net>,
Jiri Pirko <jiri@...dia.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Tony Nguyen <anthony.l.nguyen@...el.com>,
David Ahern <dsahern@...nel.org>,
Stephen Hemminger <stephen@...workplumber.org>,
linux-doc@...r.kernel.org, intel-wired-lan@...ts.osuosl.org
Subject: Re: [iproute2-next v2 3/3] devlink: add dry run attribute support to
devlink flash
Thu, Jul 21, 2022 at 11:14:51PM CEST, jacob.e.keller@...el.com wrote:
>Recent versions of the kernel support the DEVLINK_ATTR_DRY_RUN attribute
>which allows requesting a dry run of a command. A dry run is simply
>a request to validate that a command would work, without performing any
>destructive changes.
>
>The attribute is supported by the devlink flash update as a way to
>validate an update, including potentially the binary image, without
>modifying the device.
>
>Add a "dry_run" option to the command line parsing which will enable
>this attribute when requested.
>
>To avoid potential issues, only allow the attribute to be added to
>commands when the kernel recognizes it. This is important because some
>commands do not perform strict validation. If we were to add the
>attribute without this check, an old kernel may silently accept the
>command and perform an update even when dry_run was requested.
>
>Before adding the attribute, check the maximum attribute from the
>CTRL_CMD_GETFAMILY and make sure that the kernel recognizes the
>DEVLINK_ATTR_DRY_RUN attribute.
>
>Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>---
>Changes since v1
>* Make dl_kernel_supports_dry_run more generic by passing attribute
>
> devlink/devlink.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 43 insertions(+), 2 deletions(-)
>
>diff --git a/devlink/devlink.c b/devlink/devlink.c
>index 1e2cfc3d4285..24f1a70a9656 100644
>--- a/devlink/devlink.c
>+++ b/devlink/devlink.c
>@@ -296,6 +296,7 @@ static void ifname_map_free(struct ifname_map *ifname_map)
> #define DL_OPT_PORT_FN_RATE_PARENT BIT(51)
> #define DL_OPT_LINECARD BIT(52)
> #define DL_OPT_LINECARD_TYPE BIT(53)
>+#define DL_OPT_DRY_RUN BIT(54)
>
> struct dl_opts {
> uint64_t present; /* flags of present items */
>@@ -372,6 +373,8 @@ struct dl {
> bool verbose;
> bool stats;
> bool hex;
>+ bool max_attr_valid;
>+ uint32_t max_attr;
> struct {
> bool present;
> char *bus_name;
>@@ -701,6 +704,7 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
> [DEVLINK_ATTR_LINECARD_STATE] = MNL_TYPE_U8,
> [DEVLINK_ATTR_LINECARD_TYPE] = MNL_TYPE_STRING,
> [DEVLINK_ATTR_LINECARD_SUPPORTED_TYPES] = MNL_TYPE_NESTED,
>+ [DEVLINK_ATTR_DRY_RUN] = MNL_TYPE_FLAG,
> };
>
> static const enum mnl_attr_data_type
>@@ -1522,6 +1526,30 @@ static int dl_args_finding_required_validate(uint64_t o_required,
> return 0;
> }
>
>+static void dl_get_max_attr(struct dl *dl)
>+{
>+ if (!dl->max_attr_valid) {
if (dl->max_attr_valid)
return;
and then you can drop the indent.
>+ uint32_t max_attr;
>+ int err;
>+
>+ err = mnlg_socket_get_max_attr(&dl->nlg, &max_attr);
>+ if (err) {
>+ pr_err("Unable to determine maximum supported devlink attribute\n");
>+ return;
>+ }
>+
>+ dl->max_attr = max_attr;
>+ dl->max_attr_valid = true;
>+ }
>+}
>+
>+static bool dl_kernel_supports_attr(struct dl *dl, enum devlink_attr attr)
>+{
>+ dl_get_max_attr(dl);
>+
>+ return (dl->max_attr_valid && dl->max_attr >= attr);
Return is not a function. Drop the "()" here.
>+}
>+
> static int dl_argv_parse(struct dl *dl, uint64_t o_required,
> uint64_t o_optional)
> {
>@@ -2037,6 +2065,16 @@ static int dl_argv_parse(struct dl *dl, uint64_t o_required,
> dl_arg_inc(dl);
> opts->linecard_type = "";
> o_found |= DL_OPT_LINECARD_TYPE;
>+ } else if (dl_argv_match(dl, "dry_run") &&
>+ (o_all & DL_OPT_DRY_RUN)) {
>+
>+ if (!dl_kernel_supports_attr(dl, DEVLINK_ATTR_DRY_RUN)) {
>+ pr_err("Kernel does not support dry_run attribute\n");
>+ return -EOPNOTSUPP;
>+ }
>+
>+ dl_arg_inc(dl);
>+ o_found |= DL_OPT_DRY_RUN;
> } else {
> pr_err("Unknown option \"%s\"\n", dl_argv(dl));
> return -EINVAL;
>@@ -2115,6 +2153,8 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
> mnl_attr_put_strz(nlh, DEVLINK_ATTR_RATE_NODE_NAME,
> opts->rate_node_name);
> }
>+ if (opts->present & DL_OPT_DRY_RUN)
>+ mnl_attr_put(nlh, DEVLINK_ATTR_DRY_RUN, 0, NULL);
> if (opts->present & DL_OPT_PORT_TYPE)
> mnl_attr_put_u16(nlh, DEVLINK_ATTR_PORT_TYPE,
> opts->port_type);
>@@ -2326,7 +2366,7 @@ static void cmd_dev_help(void)
> pr_err(" devlink dev reload DEV [ netns { PID | NAME | ID } ]\n");
> pr_err(" [ action { driver_reinit | fw_activate } ] [ limit no_reset ]\n");
> pr_err(" devlink dev info [ DEV ]\n");
>- pr_err(" devlink dev flash DEV file PATH [ component NAME ] [ overwrite SECTION ]\n");
>+ pr_err(" devlink dev flash DEV file PATH [ component NAME ] [ overwrite SECTION ] [ dry_run ]\n");
> }
>
> static bool cmp_arr_last_handle(struct dl *dl, const char *bus_name,
>@@ -3886,7 +3926,8 @@ static int cmd_dev_flash(struct dl *dl)
> NLM_F_REQUEST | NLM_F_ACK);
>
> err = dl_argv_parse_put(nlh, dl, DL_OPT_HANDLE | DL_OPT_FLASH_FILE_NAME,
>- DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE);
>+ DL_OPT_FLASH_COMPONENT | DL_OPT_FLASH_OVERWRITE |
>+ DL_OPT_DRY_RUN);
> if (err)
> return err;
>
>--
>2.36.1
>
Powered by blists - more mailing lists