[openib-general] [PATCH] ib_mad.c: Fix request/response matching

Tue Oct 5 13:57:58 PDT 2004

On Tue, 2004-10-05 at 16:34, Sean Hefty wrote:
> >Fix endian of high tid so responses are properly matched to requests
> 
> noooooooooooooooooooo.... the TID is in the MAD and goes on the wire.
> Please, do not use CPU endian!
> 
> >                mad_send_wr->tid = ((struct ib_mad_hdr*)
> >-
> bus_to_virt(cur_send_wr->sg_list->addr))->tid;
> >+                                  bus_to_virt(cur_send_wr->sg_list->addr))-
> >>tid.id;
> 
> A response MAD should have exactly the same TID as what was sent.  Not sure
> why we aren't matching against the entire TID.

It's only done for comparison purposes (taking the TID off the wire in
network endian and converting to CPU endian);

                hi_tid = be32_to_cpu(mad->mad_hdr.tid.tid_field.hi_tid);
                list_for_each_entry(entry, &port_priv->agent_list,
agent_list) {                        if (entry->agent.hi_tid == hi_tid)
{
...

> >    if (solicited) {
> >                /* Routing is based on high 32 bits of transaction ID of MAD
> >*/
> >-               hi_tid = mad->mad_hdr.tid >> 32;
> >+               hi_tid = be32_to_cpu(mad->mad_hdr.tid.tid_field.hi_tid);
> 
> This shouldn't be necessary:

The comparison failed when the code was without the conversion.

> Sender of request (system 1):
> mad.tid = (mad_agent.hi_tid << 32) | user_tid;
> send mad
> 
> Receiver of response (system 1):
> hi_tid = mad.tid >> 32
> 
> The receiver of the request should just return the same TID that it
> received.

It does.

> >+   /*
> >+        * Leave sends with timeouts on the send list
> >+        * until either matching response is received
> >+        * or timeout occurs
> >+        */
> 
> FYI - this is about to change in my next patch.
> 
> >+union ib_tid {
> >+       u64     id;
> >+       struct {
> >+               u32     hi_tid;
> >+               u32     lo_tid;
> >+       } tid_field;
> >+};
> >+
> 
> I don't see why TID can't be u64 everywhere.  We shouldn't have to make it a
> union.

-- Hal