-
Notifications
You must be signed in to change notification settings - Fork 431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IB/RC: Initial implementation of RC transport. #19
Conversation
+ Memory mapping/unmapping. + Remote key pack/unpack. + Implement put_short() for mlx5 device. + Add unit test and performance test for put_short()
ucs_status_t (*query)(uct_pd_h pd, uct_pd_attr_t *pd_attr); | ||
|
||
ucs_status_t (*mem_map)(uct_pd_h pd, void *address, size_t length, | ||
uct_lkey_t *lkey_p); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who is providing RKEY ? I think it has to come out registration function. Am I right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it comes only from rkey_unpack
user does rkey_pack(lkey) -> OOB -> rkey_unpack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I want to use it locally, do I really want to go through pack unpack flow ? Shell we provide direct access to rkey and then have this pack/unpack option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only reason i see you would want to use it locally is for tests over loopback. and in that case we do go through pack/unpack. i see no reason adding another API just for that.
also, forcing the user do pack/unpack prevents the error of just sending the rkey "as-is" to remote peer.
Merged build triggered. |
1 similar comment
Merged build triggered. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
goto err; | ||
} | ||
|
||
dev = uct_ib_iface_device(iface); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if NULL == dev ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not allocator function, it's convenience inline function to get the IB device from the IB iface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Once testing is happy, it is good to go. |
* Add destructor error handling convention to CodeStyle * Change return value of uct_ib_device_port_check() to ucs_status_t
Merged build triggered. |
1 similar comment
Merged build triggered. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build started. |
Merged build finished. Test FAILed. |
Test FAILed. Build Log
Test FAILed. |
Merged build triggered. |
1 similar comment
Merged build triggered. |
Merged build started. |
Merged build finished. Test PASSed. |
Test PASSed. |
👍 |
IB/RC: Initial implementation of RC transport.
…nfig-max UCP: set a hard coded value for the worker's ep_config_max value - 64
…r-rndv UCP/TAG/SEND: added eager/rndv flags to tag_send op
No description provided.