Re: [PATCH 1/1] IB/rxe: drop QP0 silently

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 28, 2018 at 12:31:14PM +0800, Yanjun Zhu wrote:
>
>
> On 2018/6/28 12:27, Leon Romanovsky wrote:
> > On Thu, Jun 28, 2018 at 12:22:41PM +0800, Yanjun Zhu wrote:
> > >
> > > On 2018/6/28 9:51, Jason Gunthorpe wrote:
> > > > On Thu, Jun 28, 2018 at 09:37:39AM +0800, Yanjun Zhu wrote:
> > > > > On 2018/6/27 13:56, Leon Romanovsky wrote:
> > > > > > On Wed, Jun 27, 2018 at 11:22:18AM +0800, Yanjun Zhu wrote:
> > > > > > > On 2018/6/24 23:55, Leon Romanovsky wrote:
> > > > > > > > On Wed, Jun 20, 2018 at 11:28:27PM -0400, Zhu Yanjun wrote:
> > > > > > > > > According to "Annex A16: RDMA over Converged Ethernet (RoCE)",
> > > > > > > > >
> > > > > > > > > A16.4.3 MANAGEMENT INTERFACES
> > > > > > > > >
> > > > > > > > > As defined in the base specification, a special Queue Pair, QP0 is defined
> > > > > > > > > solely for communication between subnet manager(s) and subnet management
> > > > > > > > > agents. Since such an IB-defined subnet management architecture
> > > > > > > > > is outside the scope of this annex, it follows that there is also no
> > > > > > > > > requirement that a port which conforms to this annex be associated with
> > > > > > > > > a QP0. Thus, for end nodes designed to conform to this annex, the concept
> > > > > > > > > of QP0 is undefined and unused for any port connected to an
> > > > > > > > > Ethernet network.
> > > > > > > > > CA16-8: A packet arriving at a RoCE port containing a BTH with the
> > > > > > > > > destination QP field set to QP0 shall be silently dropped.
> > > > > > > > >
> > > > > > > > > CC: Srinivas Eeda <srinivas.eeda@xxxxxxxxxx>
> > > > > > > > > CC: Junxiao Bi <junxiao.bi@xxxxxxxxxx>
> > > > > > > > > Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxxx>
> > > > > > > > >     drivers/infiniband/sw/rxe/rxe_recv.c | 9 +++++++--
> > > > > > > > >     1 file changed, 7 insertions(+), 2 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c
> > > > > > > > > index b0c8d1e..08cb97a 100644
> > > > > > > > > +++ b/drivers/infiniband/sw/rxe/rxe_recv.c
> > > > > > > > > @@ -225,9 +225,14 @@ static int hdr_check(struct rxe_pkt_info *pkt)
> > > > > > > > >     		goto err1;
> > > > > > > > >     	}
> > > > > > > > >
> > > > > > > > > +	if (unlikely(qpn == 0)) {
> > > > > > > > > +		pr_warn_ratelimited("QP 0 is not supported\n");
> > > > > > > > Safest option is not to print this warning at all.
> > > > > > > Sorry to reply late. It is to notify the users that QP 0 is dropped. So this
> > > > > > > warning is necessary. Or else the user
> > > > > > > will take time to investigate it.
> > > > > > > In a word, this warning can save time.
> > > > > > I'm saying it from my experience of running default fedora 28 with
> > > > > > default opensm on RoCE system where I over flooded with messages
> > > > > > "ib_register_mad_agent: QP 0 not supported" from mad.c.
> > > > > >
> > > > > > I wouldn't want to see same thing for RXE too.
> > > > > Thanks.
> > > > > How about WARN_ONCE(1, "QP 0 is not supported\n");?
> > > > > This only prints once.
> > > > > This can avoid investigating and this will not print too much messages.
> > > > Why are you worried about this? No roce end point should even be able
> > > > to send qp0 traffic. There is nothing to investigate.
> > > If in some case, we need to find out where QP 0 is dropped. With this
> > > warning, we can easily find
> > > where QP 0 is dropped. Without this warning, maybe we will do some work.
> > Let's do not over engineer it.
> > WARN_ONCE will produce nice stack trace which is completely not needed.
>
> WARN_ONCE only produces "QP 0 is not supported". It will not produce stack
> trace.

WARN_ONCE() -> WARN() -> __WARN_printf() ->  warn_slowpath_fmt() ->
__warn(... NULL ) -> dump_stack()

>
> WARN_ON will produce stack trace.
>
> Thanks,
> Zhu Yanjun
> >
> > Thanks
> >
> > > Zhu Yanjun
> > > > Jason
> > > >
>

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux