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

Add RoCE support for tests #13

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from
Open

Add RoCE support for tests #13

wants to merge 3 commits into from

Conversation

ksang
Copy link

@ksang ksang commented May 19, 2017

Issue #10

Only tested loopback test as I don't have env for MPI test running on two nodes.

@@ -1,9 +1,9 @@
/*
* GPUDirect Async latency benchmark
*
*
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please address whitespace issues in a different pull req ?

@@ -193,8 +193,22 @@ static int pp_connect_ctx(struct pingpong_context *ctx, int port, int my_psn,

ctx->ah = ibv_create_ah(ctx->pd, &ah_attr);
if (!ctx->ah) {
fprintf(stderr, "Failed to create AH\n");
return 1;
union ibv_gid dgid;
Copy link
Contributor

Choose a reason for hiding this comment

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

you could add a comment describing this fallback code.
e.g.. "this code is required for RoCE V1 and/or V2 blabla"

Copy link
Contributor

Choose a reason for hiding this comment

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

@ferasd could you please review this change ?
as a context, it is required to run this test in RoCE mode.

@@ -918,8 +932,16 @@ int main(int argc, char *argv[])

ctx->ah = ibv_create_ah(ctx->pd, &ah_attr);
if (!ctx->ah) {
fprintf(stderr, "Failed to create AH\n");
return 1;
ah_attr.is_global = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

as before, add a comment here

@drossetti
Copy link
Contributor

@ksang I'm going to integrate the cleaup branch into devel. can you please rebase this change on that other branch or wait ?

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.

2 participants