Quoting r. Sean Hefty <[EMAIL PROTECTED]>:
> Subject: Re: [PATCH] mad: add GID/class checking for matching received to
> sent MADs
>
> Michael S. Tsirkin wrote:
> >>>+static inline int rcv_has_same_gid(struct ib_mad_send_wr_private *wr,
> >>>+ struct ib_mad_recv_wc *rwc )
> >>>+{
> >>>+ struct ib_ah_attr attr;
> >>>+ u8 send_resp, rcv_resp;
> >>>+
> >>>+ send_resp = ((struct ib_mad *)(wr->send_buf.mad))->
> >>>+ mad_hdr.method & IB_MGMT_METHOD_RESP;
> >>>+ rcv_resp = rwc->recv_buf.mad->mad_hdr.method & IB_MGMT_METHOD_RESP;
> >>>+
> >>>+ if (!send_resp && rcv_resp)
> >>>+ /* is request/response. GID/LIDs are both local (same). */
> >>>+ return 1;
> >>>+
> >>>+ if (send_resp == rcv_resp)
> >>>+ /* both requests, or both responses. GIDs different */
> >>>+ return 0;
> >>
> >>The two checks above are only checking response bits of a sent and
> >>received MAD. How do these relate to checking the GIDs?
> >
> >
> >Well, the comments really explain what the checks do, don't they?
> >Would rcv_has_same_initiator be a better name?
>
> The comments don't help. Returning that the GIDs match just because a send
> is a request and a receive is a response is misleading. How does the
> function know this?
We are comparing the GIDs of the requestor, remember?
I initiated one of the MADs and remote side initiated the other one
so I know they cant match.
> >>>+ if (ib_query_ah(wr->send_buf.ah, &attr))
> >>>+ /* Assume not equal, to avoid false positives. */
> >>>+ return 0;
> >>
> >>Querying for the address handle every time seems expensive.
> >
> >
> >Its a very cheap operation. Look at the implementation: the low level
> >driver already caches everything in hardware format, we just move
> >some bits around.
> >
> >
> >>Can't we save
> >>the necessary information inside the ah?
> >
> >
> >That would waste a lot of memory.
>
> If the driver is caching the memory anyway, couldn't it just store the data
> in the ah, rather than some private structure?
No, because its keeping it in hardware format.
> >Its not worth caching it. Our locking rules require for query_ah to be
> >atomic
> >which is a strong hint that it just retrieves the data cached by low level
> >driver.
>
> If the locking rules require the data to be cached by the driver, why not
> make it available to the user directly?
They dont actually require it, see. But thats what drivers happen to do.
> >We do it on receive to match ACKs to transactions.
> >I dont see how this check can be done on send.
> >Patch?
>
> I was suggesting that the query be done when sending the MAD and the
> results saved.
I dont think this can work.
> This makes less sense if the ah attributes need to be
> cached by the driver anyway.
Right.
--
Michael S. Tsirkin
Staff Engineer, Mellanox Technologies
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
