rpma: add RDMA_CM_EVENT_REJECTED event handler #802
Conversation
osalyk
force-pushed
the
rejected
branch
2 times, most recently
from
c6be400 to
1cc0ed9
Compare
osalyk
changed the title
Codecov Report
@@ Coverage Diff @@
## master #802 +/- ##
=======================================
Coverage 99.82% 99.83%
=======================================
Files 15 15
Lines 1176 1181 +5
=======================================
+ Hits 1174 1179 +5
Misses 2 2 |
osalyk
force-pushed
the
rejected
branch
3 times, most recently
from
5dca3f1 to
b0de5f2
Compare
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)
a discussion (no related file):
Please use this API at least in one simple example so we will be sure the API is complete and can be used for repeating the connection attempt.
tests/unit/conn/conn-next_event.c, line 508 at r1 (raw file):
/* * next_event__success_REJECTED - happy day scenario
Always look on the bright side of life... 🌤️
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)
osalyk
force-pushed
the
rejected
branch
2 times, most recently
from
2ef60f1 to
751b4a9
Compare
There was a problem hiding this comment.
Reviewable status: 3 of 6 files reviewed, 2 unresolved discussions (waiting on @osalyk)
examples/01-connection/client.c, line 86 at r2 (raw file):
err_conn_delete;
goto from inside the loop in no critical error situation is not good solution see my proposal in pmem/fio/198
There was a problem hiding this comment.
Reviewed 1 of 3 files at r2.
Reviewable status: 4 of 6 files reviewed, 3 unresolved discussions (waiting on @osalyk)
examples/01-connection/client.c, line 22 at r2 (raw file):
5
#define RETRY_DELAY 5
There was a problem hiding this comment.
Reviewed 2 of 3 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @osalyk)
There was a problem hiding this comment.
Reviewed 2 of 3 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @osalyk)
examples/01-connection/client.c, line 61 at r2 (raw file):
return ret; /* connect the connection request and obtain the connection object */
/* prepare a connection's private data */
examples/01-connection/client.c, line 92 at r2 (raw file):
if (rpma_conn_disconnect(conn)) { (void) rpma_conn_delete(&conn); goto err_peer_delete;
if (rpma_conn_disconnect(conn)) {
(void) rpma_conn_delete(&conn);
conn = NULL; /* just in case rpma_conn_delete will fail */
break;
} else {
if (rpma_conn_delete(&conn)) {
conn = NULL;
break;
}
}
There was a problem hiding this comment.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @osalyk)
examples/01-connection/client.c, line 101 at r2 (raw file):
retry(retry); }
if (conn == NULL)
goto err_peer_delete;
There was a problem hiding this comment.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @grom72 and @osalyk)
examples/01-connection/client.c, line 86 at r2 (raw file):
Previously, grom72 (Tomasz Gromadzki) wrote…
err_conn_delete;goto from inside the loop in no critical error situation is not good solution see my proposal in pmem/fio/198
My proposition to solve this issue is:
static inline void
conn_drop_and_delete(struct rpma_conn **conn_ptr) {
(void) rpma_conn_disconnect(conn_ptr);
(void) rpma_conn_delete(conn_ptr);
*conn_ptr = NULL;
}
/* ... */
ret = rpma_conn_next_event(conn, &conn_event);
if (ret) {
conn_drop_and_delete(&conn);
break;
} else if (conn_event == RPMA_CONN_ESTABLISHED) {
break;
} else if (conn_event == RPMA_CONN_REJECTED) {
fprintf(stderr,
"rpma_conn_next_event: %s\n",
rpma_utils_conn_event_2str(conn_event));
if (retry < MAX_RETRY - 1) {
fprintf(stderr, "The retry number exceeded. Closing.\n");
conn_drop_and_delete(&conn);
break;
} else {
fprintf(stderr, "Retrying...\n");
}
} else {
fprintf(stderr,
"rpma_conn_next_event returned an unexpected event: %s\n",
rpma_utils_conn_event_2str(conn_event));
conn_drop_and_delete(&conn);
break;
}
There was a problem hiding this comment.
Reviewed 1 of 3 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @grom72 and @osalyk)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @janekmi and @osalyk)
examples/01-connection/client.c, line 86 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
My proposition to solve this issue is:
static inline void conn_drop_and_delete(struct rpma_conn **conn_ptr) { (void) rpma_conn_disconnect(conn_ptr); (void) rpma_conn_delete(conn_ptr); *conn_ptr = NULL; } /* ... */ ret = rpma_conn_next_event(conn, &conn_event); if (ret) { conn_drop_and_delete(&conn); break; } else if (conn_event == RPMA_CONN_ESTABLISHED) { break; } else if (conn_event == RPMA_CONN_REJECTED) { fprintf(stderr, "rpma_conn_next_event: %s\n", rpma_utils_conn_event_2str(conn_event)); if (retry < MAX_RETRY - 1) { fprintf(stderr, "The retry number exceeded. Closing.\n"); conn_drop_and_delete(&conn); break; } else { fprintf(stderr, "Retrying...\n"); } } else { fprintf(stderr, "rpma_conn_next_event returned an unexpected event: %s\n", rpma_utils_conn_event_2str(conn_event)); conn_drop_and_delete(&conn); break; }
*conn_ptr = NULL; not needed
conn_drop_and_delete(&conn); shall be called every time conn_event == RPMA_CONN_REJECTED
examples/01-connection/client.c, line 90 at r2 (raw file):
if (rpma_conn_disconnect(conn)) { (void) rpma_conn_delete(&conn); goto err_peer_delete;
why is it a part of the connection establish loop
osalyk
force-pushed
the
rejected
branch
2 times, most recently
from
dd1964e to
7f131a1
Compare
There was a problem hiding this comment.
Reviewable status: 5 of 6 files reviewed, 6 unresolved discussions (waiting on @grom72, @janekmi, and @osalyk)
a discussion (no related file):
Previously, janekmi (Jan Michalski) wrote…
Please use this API at least in one simple example so we will be sure the API is complete and can be used for repeating the connection attempt.
Done.
examples/01-connection/client.c, line 22 at r2 (raw file):
Previously, grom72 (Tomasz Gromadzki) wrote…
5
#define RETRY_DELAY 5
Done.
examples/01-connection/client.c, line 61 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
/* prepare a connection's private data */
Done.
examples/01-connection/client.c, line 86 at r2 (raw file):
Previously, grom72 (Tomasz Gromadzki) wrote…
*conn_ptr = NULL;not needed
conn_drop_and_delete(&conn);shall be called every time conn_event == RPMA_CONN_REJECTED
Done.
examples/01-connection/client.c, line 92 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
if (rpma_conn_disconnect(conn)) { (void) rpma_conn_delete(&conn); conn = NULL; /* just in case rpma_conn_delete will fail */ break; } else { if (rpma_conn_delete(&conn)) { conn = NULL; break; } }
Done.
examples/01-connection/client.c, line 101 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
if (conn == NULL) goto err_peer_delete;
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @grom72 and @osalyk)
examples/01-connection/client.c, line 87 at r4 (raw file):
fprintf(stderr, "rpma_conn_next_event: %s\n", rpma_utils_conn_event_2str(conn_event));
Indentation.
examples/01-connection/client.c, line 96 at r4 (raw file):
} else { fprintf(stderr, "The retry number exceeded. Closing.\n");
This line does not fit into the previous line?
examples/01-connection/client.c, line 101 at r4 (raw file):
} else { fprintf(stderr, "rpma_conn_next_event returned an unexpected event\n");
Please use rpma_utils_conn_event_2str().
There was a problem hiding this comment.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @grom72 and @osalyk)
examples/01-connection/client.c, line 70 at r4 (raw file):
ret = rpma_conn_req_new(peer, addr, port, NULL, &req); if (ret) goto err_peer_delete;
.
examples/01-connection/client.c, line 74 at r4 (raw file):
ret = rpma_conn_req_connect(&req, &pdata, &conn); if (ret) goto err_req_delete;
.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @osalyk)
examples/01-connection/client.c, line 86 at r4 (raw file):
fprintf(stderr, "rpma_conn_next_event: %s\n", rpma_utils_conn_event_2str(conn_event));
too many lines??
There was a problem hiding this comment.
Reviewed 1 of 3 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @osalyk)
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, 2 of 3 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @grom72 and @osalyk)
examples/01-connection/client.c, line 86 at r4 (raw file):
Previously, grom72 (Tomasz Gromadzki) wrote…
fprintf(stderr, "rpma_conn_next_event: %s\n", rpma_utils_conn_event_2str(conn_event));too many lines??
wrong alignment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @grom72 and @osalyk)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @grom72, @janekmi, and @ldorau)
examples/01-connection/client.c, line 70 at r4 (raw file):
Previously, janekmi (Jan Michalski) wrote…
.
Done.
examples/01-connection/client.c, line 74 at r4 (raw file):
Previously, janekmi (Jan Michalski) wrote…
.
Done.
examples/01-connection/client.c, line 86 at r4 (raw file):
Previously, ldorau (Lukasz Dorau) wrote…
wrong alignment
Done.
examples/01-connection/client.c, line 87 at r4 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Indentation.
Done.
examples/01-connection/client.c, line 96 at r4 (raw file):
This line does not fit into the previous line?
90 characters
examples/01-connection/client.c, line 101 at r4 (raw file):
Previously, janekmi (Jan Michalski) wrote…
Please use
rpma_utils_conn_event_2str().
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @grom72 and @janekmi)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72 and @osalyk)
examples/01-connection/client.c, line 76 at r5 (raw file):
ret = rpma_conn_req_new(peer, addr, port, NULL, &req); if (ret) { conn_drop_and_delete(&conn);
No conn at this point. I think just a break is enough.
examples/01-connection/client.c, line 82 at r5 (raw file):
ret = rpma_conn_req_connect(&req, &pdata, &conn); if (ret) { conn_drop_and_delete(&conn);
No conn at this point.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72 and @janekmi)
examples/01-connection/client.c, line 76 at r5 (raw file):
Previously, janekmi (Jan Michalski) wrote…
No
connat this point. I think just abreakis enough.
Done.
examples/01-connection/client.c, line 82 at r5 (raw file):
Previously, janekmi (Jan Michalski) wrote…
No
connat this point.
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r6.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72 and @janekmi)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r6.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @osalyk)
Improve examples?
This change is