Messages in this thread | | | Date | Wed, 25 Oct 2023 09:10:46 +0300 | Subject | Re: [PATCH v2 iproute2-next 3/3] rdma: Adjust man page for rdma system set privileged_qkey command | From | Patrisious Haddad <> |
| |
On 10/24/2023 7:09 PM, Petr Machata wrote: > Patrisious Haddad <phaddad@nvidia.com> writes: > >> Signed-off-by: Patrisious Haddad <phaddad@nvidia.com> >> Reviewed-by: Michael Guralnik <michaelgur@nvidia.com> >> --- >> man/man8/rdma-system.8 | 32 +++++++++++++++++++++++++++----- >> 1 file changed, 27 insertions(+), 5 deletions(-) >> >> diff --git a/man/man8/rdma-system.8 b/man/man8/rdma-system.8 >> index ab1d89fd..a2914eb8 100644 >> --- a/man/man8/rdma-system.8 >> +++ b/man/man8/rdma-system.8 >> @@ -23,16 +23,16 @@ rdma-system \- RDMA subsystem configuration >> >> .ti -8 >> .B rdma system set >> -.BR netns >> -.BR NEWMODE >> +.BR netns/privileged_qkey >> +.BR NEWMODE/NEWSTATE > What is this netns/priveleged_qkey syntax? I thought they are > independent options. If so, the way to express it is: > > rdma system set [netns NEWMODE] [privileged_qkey NEWSTATE] > > Also, your option is not actually privileged_qkey, but privileged-qkey.
I'll fix the typo in the argument name, but about them being independent while that is true I used or,
since they can't be configured using the same command whereas your proposed syntax gives the feeling that you can
configure both in the same rdma system set command.
> >> .ti -8 >> .B rdma system help >> >> .SH "DESCRIPTION" >> -.SS rdma system set - set RDMA subsystem network namespace mode >> +.SS rdma system set - set RDMA subsystem network namespace mode or privileged qkey mode >> >> -.SS rdma system show - display RDMA subsystem network namespace mode >> +.SS rdma system show - display RDMA subsystem network namespace mode and privileged qkey state > Maybe make it just something like "configure RDMA system settings" or > whatever the umbrella term is? The next option will certainly have to do > something, this doesn't scale. > > Plus the lines are waaay over 80, even over 90 that I think I've seen > Stephen or David mention as OK for iproute2 code.
Will fix all line lengths, thanks for noting.
True about scaling, but isn't the idea behind man-page to be comprehensive and list all options explicitly ?
> >> .PP >> .I "NEWMODE" >> @@ -49,12 +49,21 @@ network namespaces is not needed, shared mode can be used. >> >> It is preferred to not change the subsystem mode when there is active >> RDMA traffic running, even though it is supported. >> +.PP >> +.I "NEWSTATE" >> +- specifies the new state of the privileged_qkey parameter. Either enabled or disabled. >> +Whereas this decides whether a non-privileged user is allowed to specify a controlled >> +QKEY or not, since such QKEYS are considered privileged. >> + >> +When this parameter is enabled, non-privileged users will be allowed to >> +specify a controlled QKEY. > This is missing syntax notes. One might think that to enable it they > need to say "enable", but in fact it's "on", and "off" for disabled. > There should be an "{on | off}" somewhere in there. > > Also, line length. > > Also, the paragraph is imho a bit long-winded. Maybe make it just this? > > determines whether a non-privileged user is allowed to specify a > controlled QKEY or not. will rewrite. > >> .SH "EXAMPLES" >> .PP >> rdma system show >> .RS 4 >> -Shows the state of RDMA subsystem network namespace mode on the system. >> +Shows the state of RDMA subsystem network namespace mode on the system and >> +the state of privileged qkey parameter. >> .RE >> .PP >> rdma system set netns exclusive >> @@ -69,6 +78,19 @@ Sets the RDMA subsystem in network namespace shared mode. In this mode RDMA devi >> are shared among network namespaces. >> .RE >> .PP >> +.PP >> +rdma system set privileged_qkey enabled >> +.RS 4 >> +Sets the privileged_qkey parameter to enabled. In this state non-privileged user >> +is allowed to specify a controlled QKEY. >> +.RE >> +.PP >> +rdma system set privileged_qkey disabled >> +.RS 4 >> +Sets the privileged_qkey parameter to disabled. In this state non-privileged user >> +is *not* allowed to specify a controlled QKEY. >> +.RE >> +.PP > on | off, not enabled | disabled. yeah I don't know how I missed that will fix in all instances of man-page. > >> .SH SEE ALSO >> .BR rdma (8),
| |