Skip to content
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

UCP: added the UCX_TLS env parameter for allowing the user to select the uct/s he wants to use. #104

Merged
merged 1 commit into from
Jan 27, 2015
Merged

UCP: added the UCX_TLS env parameter for allowing the user to select the uct/s he wants to use. #104

merged 1 commit into from
Jan 27, 2015

Conversation

alinask
Copy link
Contributor

@alinask alinask commented Jan 27, 2015

No description provided.

@mellanox-github
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com:8000/jenkins-secure/job/gh-ucx-pr/223/
Test PASSed.

@alinask
Copy link
Contributor Author

alinask commented Jan 27, 2015

@yosefe please review

/**
* Transports, sorted by priority (highest to lowest)
*/
typedef enum ucx_uct_id {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think we want to have hard-coded list of transports anywhere, it makes the infrastructure less generic.
we can just let the user pass an array of strings (same as with device names) and compare them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this list of tls is limited. wouldn't it be better to hold a predefined list of supported tls?
why is this less generic? i think it's more organized this way

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this way, in order to add new transport, need to add it to this list, instead of just adding a plug-in component.
we wanted it to be more similar to mca (and maybe have transports in separate libraries in the future).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont see why it's wrong to add this to the list. after all, ucp should be aware of the ucts it supports.
but ok, i will change this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UCP will be aware of the UCT's, but only during run-time and not in compile-time.
It uses query_resources to get all TLs and their properties.

@yosefe
Copy link
Contributor

yosefe commented Jan 27, 2015

and general comment - looks like commit line is too long. if it's too long, git cuts it off when displaying short log/reflog.

@mellanox-github
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com:8000/jenkins-secure/job/gh-ucx-pr/224/
Test PASSed.

/**
* TL specification
*/
typedef struct ucp_tl_config {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not critical, but this does not have to be separate struct, can be declared directly inside struct ucp_iface_config

It allows the user to select the uct/s he wants to use.
@mellanox-github
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com:8000/jenkins-secure/job/gh-ucx-pr/225/
Test PASSed.

@alinask
Copy link
Contributor Author

alinask commented Jan 27, 2015

@yosefe please review

@yosefe
Copy link
Contributor

yosefe commented Jan 27, 2015

👍

yosefe added a commit that referenced this pull request Jan 27, 2015
UCP: added the UCX_TLS env parameter for allowing the user to select the uct/s he wants to use.
@yosefe yosefe merged commit 3bdf257 into openucx:master Jan 27, 2015
@yosefe yosefe deleted the topic/ucp-tls-selection branch January 27, 2015 16:57
evgeny-leksikov pushed a commit to evgeny-leksikov/ucx that referenced this pull request Dec 14, 2020
UCT/IB: get roce ndev name according to right gid but not fixed gid 0
dmitrygx pushed a commit to dmitrygx/ucx that referenced this pull request Dec 1, 2021
log basic resource init/fin and transfer functions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants