[ofw] IBAL CM ignores responder resources of the passive side

[Fab Tillier]

I'd just let existing broken apps get fixed and fix it in the CM properly.  That is, do the straight assignment, not the conditional assignment.

Cheers,
-Fab

-----Original Message-----
From: Leonid Keller [mailto:leonid at mellanox.co.il] 
Sent: Wednesday, June 22, 2011 10:03 AM
To: Hefty, Sean; Fab Tillier
Cc: ofw_list
Subject: RE: IBAL CM ignores responder resources of the passive side

I'm afraid that just

p_cep->resp_res = p_cm_rep->resp_res;

can break some existing applications, which do not fill p_cm_rep->resp_res and still work.
Maybe 

if (p_cm_rep->resp_res)
	p_cep->resp_res = p_cm_rep->resp_res;

> -----Original Message-----
> From: Hefty, Sean [mailto:sean.hefty at intel.com]
> Sent: Wednesday, June 22, 2011 7:04 PM
> To: Leonid Keller; Fab Tillier
> Cc: ofw_list
> Subject: RE: IBAL CM ignores responder resources of the passive side
> 
> without looking at more of the code, it looks like a bug
> 
> 
> > __save_user_rep(
> >
> >                 IN
> > cep_agent_t* const                                       p_port_cep,
> >
> >                 IN
> > kcep_t* const
> p_cep,
> >
> >                 IN                           const     iba_cm_rep*
> const
> > p_cm_rep,
> >
> >                 IN
> > uint8_t
> > rnr_nak_timeout )
> >
> > {
> >
> >                 AL_ENTER( AL_DBG_CM );
> >
> >
> >
> >                 p_cep->local_qpn = p_cm_rep->qpn;
> >
> >                 p_cep->rq_psn = p_cm_rep->starting_psn;
> >
> >                 p_cep->init_depth = p_cm_rep->init_depth;
> >
> >
> >
> 
> 
> I think we can just always do the assignment:
> 
> 	p_cep->resp_res = p_cm_rep->resp_res;
> 
> here.  Is the p_cep->resp_res the value that actually goes into the
> REP?
> 
> 
> >                 ci_ca_lock_attr( p_port_cep->h_ca->obj.p_ci_ca );
> >
> >                 /* Check the CA's responder resource max and trim if
> > necessary. */
> >
> >                 if( p_port_cep->h_ca->obj.p_ci_ca->p_pnp_attr-
> > >max_qp_resp_res <
> 
> 
> whine: The max_qp_resp_res isn't going to dynamically change.  We
> should use a saved value somewhere that doesn't require serializing all
> CM users for this check.
> 
> - Sean