[Openib-windows] Bug or by design?

[Saul]

Ok, that makes sense.  

So are you thinking something like this instead?

at line 738 in hca_data.c

         case IB_QPT_UNRELIABLE_DGRM:
         case IB_QPT_QP0:
         case IB_QPT_QP1:
         default:
            if (qp_p->state != IB_QPS_RTS) {
               *qp_attr_mask_p |= /* required flags */
                  IB_QP_SQ_PSN;
            }


----- Original Message ----- 
From: "Leonid Keller" <leonid at mellanox.co.il>
To: "Saul" <26oo at cox.net>; "Fabian Tillier" <ftillier at silverstorm.com>
Cc: <openib-windows at openib.org>
Sent: Tuesday, August 15, 2006 1:10 PM
Subject: RE: [Openib-windows] Bug or by design?


I believe, your patch

<       t[IBQPS_RTS].req_param[UD]      = IB_QP_SQ_PSN;
<       t[IBQPS_RTS].req_param[UC]      = IB_QP_SQ_PSN;
<       t[IBQPS_RTS].req_param[RC]      = IB_QP_TIMEOUT |IB_QP_RETRY_CNT

|IB_QP_RNR_RETRY |IB_QP_SQ_PSN |IB_QP_MAX_QP_RD_ATOMIC;
<       t[IBQPS_RTS].req_param[MLX]     = IB_QP_SQ_PSN;

 is a not a best solution, because IB_QP_SQ_PSN is not a required
parameter for RTS-RTS transition.

You've pinpoint a real problem: mlnx_conv_qp_modify_attr() routine
doesn't check the type of transition.
For example, in your case it presumes, that one performs an RTR-RTS
transition and that's why it sets the required IB_QP_SQ_PSN flag.
[BTW, there is no mapping from IB_MOD_QP_xxx to IB_QP_SQ_PSN, just
because it is a required parameter]

I'm working on a patch now.
  

> -----Original Message-----
> From: Saul [mailto:26oo at cox.net] 
> Sent: Monday, August 14, 2006 9:24 PM
> To: Fabian Tillier; Leonid Keller
> Cc: openib-windows at openib.org
> Subject: Re: [Openib-windows] Bug or by design?
> 
> I later discovered that you cannot go from RTS -> RTR -> RTS, 
> so I scrapped that idea.  I made another post to the list 
> with the "patch" that I put into mthca_qp.c to fix things.  
> It appears to me that there just needed to be some required 
> params specified for the RTS->RTS state change, just like 
> there are for the RTR->RTS change.  Without anything being 
> specified as required, but with the routine 
> mlnx_conv_qp_modify_attr() forcing IB_QP_SQ_PSN to be 
> required (line 743) it was a no win situation.  If there was 
> an IB_MOD_QP_xxx flag for SQ_PSN I would be fine, but since 
> there is not I was stuck.
> 
> Please let me know if the changes that I made to mthca_qp.c 
> are incorrect.
> 
> Thanks
> 
> ----- Original Message -----
> From: "Fabian Tillier" <ftillier at silverstorm.com>
> To: "Saul" <26oo at cox.net>; "Leonid Keller" <leonid at mellanox.co.il>
> Cc: <openib-windows at openib.org>
> Sent: Monday, August 14, 2006 12:23 PM
> Subject: Re: [Openib-windows] Bug or by design?
> 
> 
> > Hi Saul,
> >
> > On 8/12/06, Saul <26oo at cox.net> wrote:
> >>
> >> Hello,
> >>
> >> I am having a problem updating the qkey of a QP that is in 
> the RTS state.
> >> The API I'm using is
> >>
> >> ib_modify_qp(const ib_qp_handle_t h_qp, const ib_qp_mod_t *const 
> >> p_qp_mod);
> >>
> >> I set qp_mod.req_state (to IB_QPS_RTS), 
> qp_mod.state.rts.qkey (to my new
> >> qkey) and qp_mod.state.rts.opts (to IB_MOD_QP_KEY).
> >>
> >> The problem is that down in the mthca driver it uses the 
> integer style
> >> attribute mask.  In the routine mlnx_conv_qp_modify_attr() 
> the code adds 
> >> the
> >> attribute flag IB_QP_SQ_PSN as a required flag.  I can set 
> this field in
> >> qp_mod without any problem but there is no IB_MOD_QP_xxx 
> flag that maps 
> >> to
> >> IB_QP_SQ_PSN.  Therefore the check at line 562 in 
> mthca_qp.c fails the
> >> operation.
> >>
> >> So my question is:  is this supposed to operate this way?  
> Can I not call
> >> ib_modify_qp() while at RTS if I want to stay at RTS?  I 
> would think that
> >> this is ok because it is not flagged as an invalid state 
> transition in 
> >> the
> >> driver.  Is there some other API that I should use to 
> perform this qkey
> >> update?  Or do I need to transition to RTR, then back to 
> RTS to avoid 
> >> this?
> >
> > This should work.  You can't move back from RTS to RTR without
> > resetting the QP first and moving it back through INIT, which will
> > break your connection.
> >
> > Leonid, can you look into this?
> >
> > Thanks,
> >
> > - Fab
> > 
> 
>