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: <d1b8b5b4-d9ae-ca88-4372-e62988998634@nvidia.com> Date: Sun, 22 Oct 2023 12:22:03 +0300 From: Patrisious Haddad <phaddad@...dia.com> To: Petr Machata <petrm@...dia.com> Cc: jgg@...pe.ca, leon@...nel.org, dsahern@...il.com, stephen@...workplumber.org, netdev@...r.kernel.org, linux-rdma@...r.kernel.org, linuxarm@...wei.com, linux-kernel@...r.kernel.org, huangjunxian6@...ilicon.com, michaelgur@...dia.com Subject: Re: [PATCH iproute2-next 2/3] rdma: Add an option to set privileged QKEY parameter On 10/19/2023 1:38 PM, Petr Machata wrote: > Patrisious Haddad <phaddad@...dia.com> writes: > >> @@ -40,6 +45,22 @@ static int sys_show_parse_cb(const struct nlmsghdr *nlh, void *data) >> mode_str); >> } >> >> + if (tb[RDMA_NLDEV_SYS_ATTR_PRIVILEGED_QKEY_MODE]) { >> + const char *pqkey_str; >> + uint8_t pqkey_mode; >> + >> + pqkey_mode = >> + mnl_attr_get_u8(tb[RDMA_NLDEV_SYS_ATTR_PRIVILEGED_QKEY_MODE]); >> + >> + if (pqkey_mode < ARRAY_SIZE(privileged_qkey_str)) >> + pqkey_str = privileged_qkey_str[pqkey_mode]; >> + else >> + pqkey_str = "unknown"; >> + >> + print_color_string(PRINT_ANY, COLOR_NONE, "privileged-qkey", >> + "privileged-qkey %s ", pqkey_str); >> + } >> + > Elsewhere in the file, you just use print_color_on_off(), why not here? About this as I previously answered I don't really see a big difference between it and "print_color_string" but if the maintainer thinks this is an essential change I can fix and re-send. Thanks for the review. > >> if (tb[RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK]) >> cof = mnl_attr_get_u8(tb[RDMA_NLDEV_SYS_ATTR_COPY_ON_FORK]); >> >> @@ -111,10 +155,25 @@ static int sys_set_netns_args(struct rd *rd) >> return sys_set_netns_cmd(rd, cmd); >> } >> >> +static int sys_set_privileged_qkey_args(struct rd *rd) >> +{ >> + bool cmd; >> + >> + if (rd_no_arg(rd) || !sys_valid_privileged_qkey_cmd(rd_argv(rd))) { >> + pr_err("valid options are: { on | off }\n"); >> + return -EINVAL; >> + } > This could use parse_on_off(). More importantly I looked a bit more into it, and I prefer not to use it, it would also lead to additional error prints that are not consistent with what we previously had for this API, so I prefer to keep it as is , so that the error messages for all arguments of this command be identical. > >> + >> + cmd = (strcmp(rd_argv(rd), "on") == 0) ? true : false; >> + >> + return sys_set_privileged_qkey_cmd(rd, cmd); >> +} >> + >> static int sys_set_help(struct rd *rd) >> { >> pr_out("Usage: %s system set [PARAM] value\n", rd->filename); >> pr_out(" system set netns { shared | exclusive }\n"); >> + pr_out(" system set privileged-qkey { on | off }\n"); >> return 0; >> } >> >> @@ -124,6 +183,7 @@ static int sys_set(struct rd *rd) >> { NULL, sys_set_help }, >> { "help", sys_set_help }, >> { "netns", sys_set_netns_args}, >> + { "privileged-qkey", sys_set_privileged_qkey_args}, >> { 0 } >> }; > The rest of the code looks sane to me, but I'm not familiar with the > feature.
Powered by blists - more mailing lists