[openib-general] [PATCH 2/2] ib_cm: fix REJ due to invalid GID

Arne Redlich arne.redlich at xiranet.com
Tue Jul 18 08:09:06 PDT 2006


Am Dienstag, den 18.07.2006, 17:43 +0300 schrieb Michael S. Tsirkin:
> Quoting r. Arne Redlich <arne.redlich at xiranet.com>:
> > Reject a REQ containing invalid GID(s) with appropriate reason codes
> > instead of IB_CM_CONSUMER_REJ.
> > 
> > Signed-off-by: Arne Redlich <arne.redlich at xiranet.com>
> 
> Are there actual applications that can use the exact reject status? 

Yep - the Gen1 SRP initiator does. It sends a REQ with an invalid DGID.
If rejected with the correct code (INVALID GID), it will retry after
looking up the GID.

>  I also
> wander what should the status be if there are multiple errors in a REQ.

The first one discovered, no?

> > @@ -1373,12 +1375,21 @@ static int cm_req_handler(struct cm_work
> >  
> >  	cm_format_paths_from_req(req_msg, &work->path[0], &work->path[1]);
> >  	ret = cm_init_av_by_path(&work->path[0], &cm_id_priv->av);
> > -	if (ret)
> > +	if (ret) {
> > +		cm_issue_rej(work->port, work->mad_recv_wc, IB_CM_INVALID_GID,
> > +			     CM_MSG_RESPONSE_REQ, NULL, 0);
> > +		reject = 0;
> >  		goto error3;
> > +	}
> >  	if (req_msg->alt_local_lid) {
> >  		ret = cm_init_av_by_path(&work->path[1], &cm_id_priv->alt_av);
> > -		if (ret)
> > +		if (ret) {
> > +			cm_issue_rej(work->port, work->mad_recv_wc,
> > +				     IB_CM_INVALID_ALT_GID, CM_MSG_RESPONSE_REQ,
> > +				     NULL, 0);
> > +			reject = 0;
> >  			goto error3;
> > +		}
> >  	}
> 
> Hmm ... it looks like cm_init_av_by_path can also fail if ib_find_cached_pkey
> returns an error - is it right that your patch will return invalid gid
> in this case?
> 
> Maybe the right thing to do is to
> 1. Make cm_init_av_by_path return a more specific error in case of GID mismatch.
>    ENXIO might be a good fit, but we can always add our own

Agreed. I've indeed overlooked it in the initial patch.

> 2. Teach cm_destroy_id to send invalid gid reject on this error

Hmm, I'd rather agree with Roland that passing a rej reason to
cm_destroy_id() would be the cleanest way (while I attempted to go the
easiest) - my brain cannot cope with too many layers of indirection that
easily. ;-)

Thanks,
Arne
-- 
Arne Redlich
Xiranet Communications GmbH





More information about the general mailing list