RE: [RDMA for-next V3 1/6] IB/MAD: Add send path trace points

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

 



> On Wed, Dec 26, 2018 at 08:55:26PM -0500, ira.weiny@xxxxxxxxx wrote:
> > +DECLARE_EVENT_CLASS(ib_mad_send_template,
> > +	TP_PROTO(struct ib_mad_send_wr_private *wr, struct
> rdma_mad_trace_addr *addr),
> > +	TP_ARGS(wr, addr),
> > +
> > +	TP_STRUCT__entry(
> > +		__field(u8,             base_version)
> > +		__field(u8,             mgmt_class)
> > +		__field(u8,             class_version)
> > +		__field(u8,             port_num)
> > +		__field(u32,            qp_num)
> > +		__field(u8,             method)
> > +		__field(u8,             sl)
> > +		__field(u16,            attr_id)
> > +		__field(u32,            attr_mod)
> > +		__field(u64,            wrtid)
> > +		__field(u64,            tid)
> > +		__field(u16,            status)
> > +		__field(u16,            class_specific)
> > +		__field(u32,            length)
> > +		__field(u32,            dlid)
> > +		__field(u32,            rqpn)
> > +		__string(dev_name,      wr->mad_agent_priv->agent.device-
> >name)
> > +		__field(void *,         agent_priv)
> > +		__field(unsigned long,  timeout)
> > +		__field(int,            retries_left)
> > +		__field(int,            max_retries)
> > +		__field(int,            retry)
> > +		__field(u32,            rqkey)
> > +		__field(u16,            pkey)
> > +	),
> 
> Does it really make sense to extract all this data? Can we just trace the bulk
> packet header? I have no idea what convention is in tracing..

There is a fine line between tracing packets and printing enough information to "ID the MAD".  That is part of the reason the RMPP headers are not decoded.  I did not want to go that far into the packet parsing.

There are 2 general things I think are critical to "tracing the MAD stack".  1) something to ID the "MAD" as it flows through the stack.  2) data which is indicative of what is happening (retry, timeout, send failure vs no response) etc.

1) we need a min of QPN, Class (or agent info), TID (MAD assigned and user assigned), device name (or ID; I'll change this), and port number.

2) includes the timeout, status, destination info (rqpn/dlid), retry info (left, max retry, retry count), agent info.

That does not leave much "extra".  pkey, rqkey, class_specific (could be part of SMP decoding), length (although that is pretty useful), base_version, attribute ID, attribute modifier, method, sl, and class_version (but in OPA class version helps).

I will concede that a couple of these are not really useful like base_version as it basically should never change.  Pkey and class_version are pretty useful.

I agree that there is a line here which I'm drawing pretty far towards the "packet capture" side of things.  But remember originally part of the goal was to get rid of the snoop interface.  "madeye" the main user of that interface did decode all these fields.

Tracing the bulk packet header may be better but it would only cut down the fields by about 1/3.  The downside is for users who are not familiar with MADs decoding the header can be a pain so I really feel like leaving those decodes in.

Thoughts,
Ira

> 
> Jason



[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