| Message ID | 00fe01d3ec4f$44a6f400$cdf4dc00$@opengridcomputing.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Tue, May 15, 2018 at 08:18:51AM -0500, Steve Wise wrote: > > > > On Mon, May 14, 2018 at 05:04:26PM -0500, Steve Wise wrote: > > > > > > > > > > > > On 5/14/2018 3:41 PM, Jason Gunthorpe wrote: > > > > > On Mon, May 07, 2018 at 08:53:16AM -0700, Steve Wise wrote: > > > > >> This enhancement allows printing rdma device-specific state, if > > provided > > > > >> by the kernel. This is done in a generic manner, so rdma tool > doesn't > > > > >> need to know about the details of every type of rdma device. > > > > >> > > > > >> Driver attributes for a rdma resource are in the form of <key, > > > > >> [print_type], value> tuples, where the key is a string and the > value can > > > > >> be any supported driver attribute. The print_type attribute, if > present, > > > > >> provides a print format to use vs the standard print format for the > > type. > > > > >> For example, the default print type for a PROVIDER_S32 value is "%d > ", > > > > >> but "0x%x " if the print_type of PRINT_TYPE_HEX is included inthe > > tuple. > > > > >> > > > > >> Driver resources are only printed when the -dd flag is present. > > > > >> If -p is present, then the output is formatted to not exceed 80 > > columns, > > > > >> otherwise it is printed as a single row to be grep/awk friendly. > > > > >> > > > > >> Example output: > > > > >> > > > > >> # rdma resource show qp lqpn 1028 -dd -p > > > > >> link cxgb4_0/- lqpn 1028 rqpn 0 type RC state RTS rq-psn 0 sq-psn 0 > > > path-mig-state MIGRATED pid 0 comm [nvme_rdma] > > > > >> sqid 1028 flushed 0 memsize 123968 cidx 85 pidx 85 wq_pidx 106 > > > flush_cidx 85 in_use 0 > > > > >> size 386 flags 0x0 rqid 1029 memsize 16768 cidx 43 pidx 41 > wq_pidx > > > 171 msn 44 rqt_hwaddr 0x2a8a5d00 > > > > >> rqt_size 256 in_use 128 size 130 idx 43 wr_id > 0xffff881057c03408 idx > > > 40 wr_id 0xffff881057c033f0 > > > > > Hey some of these look like kernel pointers.. That is a no-no.. What > > > > > is up there? > > > > > > > > Nothing is defined as a kernel pointer. But wr_id is often a pointer > to > > > > the kernel rdma application's context... > > > > > > > > > The wr_id often contains a pointer, right? So we cannot just pass it > > > > > to user space.. > > > > > > > > Hmm. It is useful for debugging kernel rdma applications. Perhaps > > > > these attrs can be only be sent up by the kernel if the capabilities > > > > allow. But previous review comments of the kernel series, which is > now > > > > merged, forced me to remove passing the capabilities information to > the > > > > driver resource fill functions. > > > > > > > > So what's the right way to do this? > > > > > > The reviewer asked do not pass to drivers whole CAP_.. bits, because > > > they anyway don't need such granularity. > > > > > > > Ok thanks. > > How's this? > > diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h > index 6379685..2cf9c5c 100644 > +++ b/include/rdma/restrack.h > @@ -66,7 +66,8 @@ struct rdma_restrack_root { > * Allows rdma drivers to add their own restrack attributes. > */ > int (*fill_res_entry)(struct sk_buff *msg, > - struct rdma_restrack_entry *entry); > + struct rdma_restrack_entry *entry, > + bool net_admin_capable); > }; cap net admin is not high enough privledge to see unhashed kernel pointers. CAP_RAW_IO? Or follow what printk does? Honestly, I don't really know, this hashed pointer stuff is new.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> From: Jason Gunthorpe <jgg@ziepe.ca> > On Tue, May 15, 2018 at 08:18:51AM -0500, Steve Wise wrote: > > > > > > On Mon, May 14, 2018 at 05:04:26PM -0500, Steve Wise wrote: > > > > > > > > > > > > > > > On 5/14/2018 3:41 PM, Jason Gunthorpe wrote: > > > > > > On Mon, May 07, 2018 at 08:53:16AM -0700, Steve Wise wrote: > > > > > >> This enhancement allows printing rdma device-specific state, if > > > provided > > > > > >> by the kernel. This is done in a generic manner, so rdma tool > > doesn't > > > > > >> need to know about the details of every type of rdma device. > > > > > >> > > > > > >> Driver attributes for a rdma resource are in the form of <key, > > > > > >> [print_type], value> tuples, where the key is a string and the > > value can > > > > > >> be any supported driver attribute. The print_type attribute, if > > present, > > > > > >> provides a print format to use vs the standard print format for the > > > type. > > > > > >> For example, the default print type for a PROVIDER_S32 value is > "%d > > ", > > > > > >> but "0x%x " if the print_type of PRINT_TYPE_HEX is included inthe > > > tuple. > > > > > >> > > > > > >> Driver resources are only printed when the -dd flag is present. > > > > > >> If -p is present, then the output is formatted to not exceed 80 > > > columns, > > > > > >> otherwise it is printed as a single row to be grep/awk friendly. > > > > > >> > > > > > >> Example output: > > > > > >> > > > > > >> # rdma resource show qp lqpn 1028 -dd -p > > > > > >> link cxgb4_0/- lqpn 1028 rqpn 0 type RC state RTS rq-psn 0 sq-psn 0 > > > > path-mig-state MIGRATED pid 0 comm [nvme_rdma] > > > > > >> sqid 1028 flushed 0 memsize 123968 cidx 85 pidx 85 wq_pidx 106 > > > > flush_cidx 85 in_use 0 > > > > > >> size 386 flags 0x0 rqid 1029 memsize 16768 cidx 43 pidx 41 > > wq_pidx > > > > 171 msn 44 rqt_hwaddr 0x2a8a5d00 > > > > > >> rqt_size 256 in_use 128 size 130 idx 43 wr_id > > 0xffff881057c03408 idx > > > > 40 wr_id 0xffff881057c033f0 > > > > > > Hey some of these look like kernel pointers.. That is a no-no.. What > > > > > > is up there? > > > > > > > > > > Nothing is defined as a kernel pointer. But wr_id is often a pointer > > to > > > > > the kernel rdma application's context... > > > > > > > > > > > The wr_id often contains a pointer, right? So we cannot just pass it > > > > > > to user space.. > > > > > > > > > > Hmm. It is useful for debugging kernel rdma applications. Perhaps > > > > > these attrs can be only be sent up by the kernel if the capabilities > > > > > allow. But previous review comments of the kernel series, which is > > now > > > > > merged, forced me to remove passing the capabilities information to > > the > > > > > driver resource fill functions. > > > > > > > > > > So what's the right way to do this? > > > > > > > > The reviewer asked do not pass to drivers whole CAP_.. bits, because > > > > they anyway don't need such granularity. > > > > > > > > > > Ok thanks. > > > > How's this? > > > > diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h > > index 6379685..2cf9c5c 100644 > > +++ b/include/rdma/restrack.h > > @@ -66,7 +66,8 @@ struct rdma_restrack_root { > > * Allows rdma drivers to add their own restrack attributes. > > */ > > int (*fill_res_entry)(struct sk_buff *msg, > > - struct rdma_restrack_entry *entry); > > + struct rdma_restrack_entry *entry, > > + bool net_admin_capable); > > }; > > cap net admin is not high enough privledge to see unhashed kernel > pointers. CAP_RAW_IO? Or follow what printk does? > Do you mean CAP_NET_RAW? Here's the comments for it: /* Allow use of RAW sockets */ /* Allow use of PACKET sockets */ /* Allow binding to any address for transparent proxying (also via NET_ADMIN) */ Func restricted_pointer() from lib/vsprintf.c uses CAP_SYSLOG. The comment for CAP_SYSLOG: /* Allow configuring the kernel's syslog (printk behaviour) */ Func kallsyms_show_value() also uses CAP_SYSLOG. And there is a non-exported global kptr_restrict that they use apparently to allow overriding all this for profiling. Here is NET_ADMIN's comments: /* Allow interface configuration */ /* Allow administration of IP firewall, masquerading and accounting */ /* Allow setting debug option on sockets */ /* Allow modification of routing tables */ /* Allow setting arbitrary process / process group ownership on sockets */ /* Allow binding to any address for transparent proxying (also via NET_RAW) */ /* Allow setting TOS (type of service) */ /* Allow setting promiscuous mode */ /* Allow clearing driver statistics */ /* Allow multicasting */ /* Allow read/write of device-specific registers */ /* Allow activation of ATM control sockets */ -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 15, 2018 at 09:31:27AM -0500, Steve Wise wrote: > > cap net admin is not high enough privledge to see unhashed kernel > > pointers. CAP_RAW_IO? Or follow what printk does? > > > > Do you mean CAP_NET_RAW? Here's the comments for it: Nope.. > Func restricted_pointer() from lib/vsprintf.c uses CAP_SYSLOG. The comment for CAP_SYSLOG: Yikes, yes, that is probably the required logic here, including the kptr_restrict = 0 thing Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Tue, May 15, 2018 at 09:31:27AM -0500, Steve Wise wrote: > > > cap net admin is not high enough privledge to see unhashed kernel > > > pointers. CAP_RAW_IO? Or follow what printk does? > > > > > > > Do you mean CAP_NET_RAW? Here's the comments for it: > > Nope.. > > > Func restricted_pointer() from lib/vsprintf.c uses CAP_SYSLOG. The > comment for CAP_SYSLOG: > > Yikes, yes, that is probably the required logic here, including the > kptr_restrict = 0 thing > Let's defer the ktpr_restrict issue for now; I want to finish the initial work this cycle, and adding that will likely take too much time. I'll use CAP_SYSLOG and add a FIXME comment. Ok? Steve. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 15, 2018 at 10:02:08AM -0500, Steve Wise wrote: > > On Tue, May 15, 2018 at 09:31:27AM -0500, Steve Wise wrote: > > > > cap net admin is not high enough privledge to see unhashed kernel > > > > pointers. CAP_RAW_IO? Or follow what printk does? > > > > > > > > > > Do you mean CAP_NET_RAW? Here's the comments for it: > > > > Nope.. > > > > > Func restricted_pointer() from lib/vsprintf.c uses CAP_SYSLOG. The > > comment for CAP_SYSLOG: > > > > Yikes, yes, that is probably the required logic here, including the > > kptr_restrict = 0 thing > > > > Let's defer the ktpr_restrict issue for now; I want to finish the initial > work this cycle, and adding that will likely take too much time. I'll use > CAP_SYSLOG and add a FIXME comment. Ok? Lets just drop wr_id instead... Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/rdma/restrack.h b/include/rdma/restrack.h index 6379685..2cf9c5c 100644 --- a/include/rdma/restrack.h +++ b/include/rdma/restrack.h @@ -66,7 +66,8 @@ struct rdma_restrack_root { * Allows rdma drivers to add their own restrack attributes. */ int (*fill_res_entry)(struct sk_buff *msg, - struct rdma_restrack_entry *entry); + struct rdma_restrack_entry *entry, + bool net_admin_capable); }; -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in