RE: [RFC 2/2] IB/umad: Export mad snooping capability to userspace

From: "Hefty, Sean" <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Linux RDMA list <linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: RE: [RFC 2/2] IB/umad: Export mad snooping capability to userspace
Date: Thu, 14 Oct 2010 08:20:07 -0700	[thread overview]
Message-ID: <CF9C39F99A89134C9CF9C4CCB68B8DDF25B7E5E984@orsmsx501.amr.corp.intel.com> (raw)
In-Reply-To: <AANLkTimGjm9ZzWsav91CHSqgftxTzZW3iZN1fAp+EWd5-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2344 bytes --]

> > Export the mad snooping capability to user space clients
> > through the existing umad interface. Â This will allow
> > users to capture MAD data for debugging, plus it allows
> > for services to act on MAD traffic that occurs.
> 
> Should it also be noted that snooping of RMPP'd MADs occurs at the
> reassembled packet level with the last header being indicated ?

I thought that the mad layer ending up reporting snooped MADs before reassembly and after segmentation.  It's been a while since I've used madeye, but that's what I remember.  I guess I'll find out during testing and make whatever adjustments are necessary, but I agree that this should be documented.

On a side note, while writing up this patch, it looks like madeye handles the data portion of sent RMPP MADs incorrectly.  It doesn't reference the data portion (beyond the RMPP/MAD headers) correctly.  See the snoop_send_handler for how I think this needs to be handled.

> > diff --git a/include/rdma/ib_user_mad.h b/include/rdma/ib_user_mad.h
> > index d6fce1c..0c40861 100644
> > --- a/include/rdma/ib_user_mad.h
> > +++ b/include/rdma/ib_user_mad.h
> > @@ -165,10 +165,36 @@ struct ib_user_mad {
> > Â typedef unsigned long __attribute__((aligned(4))) packed_ulong;
> > Â #define IB_USER_MAD_LONGS_PER_METHOD_MASK (128 / (8 * sizeof (long)))
> >
> > +enum {
> > + Â  Â  Â  IB_UMAD_QP0,
> > + Â  Â  Â  IB_UMAD_QP1,
> > + Â  Â  Â  IB_UMAD_SNOOP = 0x80,
> > + Â  Â  Â  IB_UMAD_SNOOP_QP0 = IB_UMAD_SNOOP & IB_UMAD_QP0,
> > + Â  Â  Â  IB_UMAD_SNOOP_QP1 = IB_UMAD_SNOOP & IB_UMAD_QP1
> > +};
> 
> typos above: s.b. | r.t. &

yep - thanks

> > + * @qpn - Queue pair number; must be 0 or 1. Â If the upper bit of the
> qpn
> > + * Â  is set to 1, then the registration will be set to snoop traffic
> > + * Â  over that QP.
> 
> Should this take half the qpn space or be two specific values ?

Two specific values is probably better.  Note that the umad interface specifies the qpn using 8-bits, so we're definitely limited on the qpns that we could ever support.  I know there's been discussion about possibly adding more reserved QPs, so I'm not sure what the best two values to reserve would be.

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±ÙšŠ{ayº\x1dÊ‡Ú™ë,j\a¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš