On Wed, 2005-05-25 at 11:38, James Lentini wrote: > halr> On Tue, 2005-05-24 at 15:20, James Lentini wrote: > halr> > halr> [kdapl CM] Fix endian conversions of service ID > halr> > halr> Problem pointed out by James Lentini > halr> > halr> > halr> > halr> Signed-off-by: Hal Rosenstock <[EMAIL PROTECTED]> > halr> > halr> > halr> > halr> Index: dapl_openib_cm.c > halr> > halr> > =================================================================== > halr> > halr> -- dapl_openib_cm.c (revision 2468) > halr> > halr> +++ dapl_openib_cm.c (working copy) > halr> > halr> @@ -309,7 +309,7 @@ > halr> > halr> if (conn->dapl_path.mtu > IB_MTU_1024) > halr> > halr> conn->dapl_path.mtu = IB_MTU_1024; > halr> > halr> > halr> > halr> - conn->param.service_id = be64_to_cpu(conn->service_id); > halr> > halr> + conn->param.service_id = conn->service_id; > halr> > > halr> > With the change to dapl_ib_connect below, the conn->service_id is in > halr> > CPU byte order at this point. The conn->param is a ib_cm_req_param > halr> > structure. The comment describing this structure's service_id field > halr> > says that it should be in network (big endian) byte order. > halr> > > halr> > So... > halr> > > halr> > halr> conn->param.primary_path = &conn->dapl_path; > halr> > halr> conn->param.alternate_path = NULL; > halr> > halr> > halr> > halr> @@ -445,8 +445,7 @@ > halr> > halr> conn->param.local_cm_response_timeout = > halr> > halr> DAPL_OPENIB_CM_RESPONSE_TIMEOUT; > halr> > halr> conn->param.max_cm_retries = DAPL_OPENIB_MAX_CM_RETRIES; > halr> > halr> > halr> > halr> - memcpy(&conn->service_id, &remote_conn_qual, sizeof > halr> > halr> conn->service_id); > halr> > halr> - > halr> > halr> + conn->service_id = be64_to_cpu(remote_conn_qual); > halr> > > halr> > ...that makes me think we should change the line above to > halr> > > halr> > conn->service_id = remote_conn_qual; > halr> > > halr> > and require that consumer's specify their connection qualifier values > halr> > in network byte order here and ... > halr> > halr> I think the convention OpenIB has been using is to supply parameters in > halr> CPU endian but it can work either way. > > The comments in ib_cm.h say that service id parameters should be in > network byte order. Are these incorrect?
No. I was referring to your alternative of where this is performed. > halr> > halr> > halr> conn->remote_ia_address = remote_ia_address; > halr> > halr> conn->dapl_comp.fn = &dapl_rt_comp_handler; > halr> > halr> conn->dapl_comp.context = conn; > halr> > halr> @@ -627,7 +626,7 @@ > halr> > halr> } > halr> > halr> > halr> > halr> status = ib_cm_listen(sp_ptr->cm_srvc_handle, > halr> > halr> - be64_to_cpu(sp_ptr->conn_qual), > 0); > halr> > halr> + cpu_to_be64(sp_ptr->conn_qual), > 0); > halr> > > halr> > ... do the same here. What do you think? > halr> > halr> _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
