All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Steve Wise" <swise@opengridcomputing.com>
To: 'Jason Gunthorpe' <jgg@ziepe.ca>
Cc: 'Leon Romanovsky' <leon@kernel.org>,
	dsahern@gmail.com, stephen@networkplumber.org,
	netdev@vger.kernel.org, linux-rdma@vger.kernel.org
Subject: RE: [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes
Date: Tue, 15 May 2018 09:31:27 -0500	[thread overview]
Message-ID: <012201d3ec59$69242fa0$3b6c8ee0$@opengridcomputing.com> (raw)
In-Reply-To: <20180515135335.GA5615@ziepe.ca>


> 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 */

WARNING: multiple messages have this Message-ID (diff)
From: "Steve Wise" <swise@opengridcomputing.com>
To: "'Jason Gunthorpe'" <jgg@ziepe.ca>
Cc: "'Leon Romanovsky'" <leon@kernel.org>, <dsahern@gmail.com>,
	<stephen@networkplumber.org>, <netdev@vger.kernel.org>,
	<linux-rdma@vger.kernel.org>
Subject: RE: [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes
Date: Tue, 15 May 2018 09:31:27 -0500	[thread overview]
Message-ID: <012201d3ec59$69242fa0$3b6c8ee0$@opengridcomputing.com> (raw)
In-Reply-To: <20180515135335.GA5615@ziepe.ca>


> 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 */

  reply	other threads:[~2018-05-15 14:31 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-07 16:06 [PATCH v1 iproute2-next 0/3] RDMA tool driver-specific resource tracking Steve Wise
2018-05-07 15:53 ` [PATCH v1 iproute2-next 1/3] rdma: update rdma_netlink.h to get driver attrs Steve Wise
2018-05-13 13:15   ` Leon Romanovsky
2018-05-14 15:15     ` Steve Wise
2018-05-07 15:53 ` [PATCH v1 iproute2-next 2/3] rdma: print driver resource attributes Steve Wise
2018-05-10  4:08   ` David Ahern
2018-05-10 14:19     ` Steve Wise
2018-05-10 14:20       ` David Ahern
2018-05-13 13:10         ` Leon Romanovsky
2018-05-13 13:24   ` Leon Romanovsky
2018-05-14 14:51     ` Steve Wise
2018-05-15 16:35       ` Doug Ledford
2018-05-15 16:59         ` Leon Romanovsky
2018-05-15 17:51           ` Steve Wise
2018-05-15 17:51             ` Steve Wise
2018-05-15 18:00           ` Doug Ledford
2018-05-14 20:41   ` Jason Gunthorpe
2018-05-14 22:04     ` Steve Wise
2018-05-15  8:54       ` Leon Romanovsky
2018-05-15 13:04         ` Steve Wise
2018-05-15 13:04           ` Steve Wise
2018-05-15 13:18         ` Steve Wise
2018-05-15 13:18           ` Steve Wise
2018-05-15 13:53           ` Jason Gunthorpe
2018-05-15 14:31             ` Steve Wise [this message]
2018-05-15 14:31               ` Steve Wise
2018-05-15 14:44               ` Jason Gunthorpe
2018-05-15 15:02                 ` Steve Wise
2018-05-15 15:02                   ` Steve Wise
2018-05-15 15:14                   ` Jason Gunthorpe
2018-05-07 15:53 ` [PATCH v1 iproute2-next 3/3] rdma: update man pages Steve Wise

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='012201d3ec59$69242fa0$3b6c8ee0$@opengridcomputing.com' \
    --to=swise@opengridcomputing.com \
    --cc=dsahern@gmail.com \
    --cc=jgg@ziepe.ca \
    --cc=leon@kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.