rpma: implement separate receive CQ #1080

yangx-jy · 2021-06-03T12:53:02Z

This change is

codecov · 2021-06-03T12:57:22Z

Codecov Report

Merging #1080 (66b95b7) into master (a2a496d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1080   +/-   ##
=======================================
  Coverage   99.84%   99.84%           
=======================================
  Files          16       16           
  Lines        1264     1321   +57     
=======================================
+ Hits         1262     1319   +57     
  Misses          2        2

janekmi

Reviewed 23 of 59 files at r1.
Reviewable status: 7 of 59 files reviewed, 10 unresolved discussions (waiting on @janekmi and @yangx-jy)

src/conn.c, line 258 at r1 (raw file):

	rdma_destroy_qp(conn->id);

	ret = rpma_cq_delete(&conn->rcq);

cq first before rcq.

src/conn.c, line 479 at r1 (raw file):

/*
 * rpma_conn_get_cq -- get a rpma_cq object from the connection

get the connection's send CQ

src/conn.c, line 493 at r1 (raw file):

/*
 * rpma_conn_get_rcq -- get a receiving rpma_cq object from the connection

get the connection's receive CQ

src/conn_cfg.c, line 62 at r1 (raw file):

/*
 * Round down the cq_size when it is too big for storing

/*
 * rpma_conn_cfg... -- ...

src/conn_cfg.c, line 65 at r1 (raw file):

 * into an int type of value. Convert otherwise.
 */
static int rpma_round_down_size(uint32_t size)

static int
rpma_conn_cfg_...

src/conn_cfg.c, line 71 at r1 (raw file):

	return (int)size;
}

It is not on the internal API of this unit so it should go above the internal API boundary.

TBH I think it may be an even better fit into common.h as a simple macro? Up to you.

src/conn_cfg.c, line 78 at r1 (raw file):

 */
int
rpma_conn_cfg_get_cqe(const struct rpma_conn_cfg *cfg, int *cqe, int *recv_cqe)

I know it is tiresome to have getters and setters for each of the fields separately but I believe it makes the API easier to maintain in the future. The rpma_conn_cfg_get_cqe() function is a variant of rpma_conn_cfg_get_cq_size() and I think it should stay this way. Instead, I think you should introduce something like rpma_conn_cfg_get_rcqe() (following the naming conventions).

src/cq.c, line 165 at r1 (raw file):

{
	return cq->cq;
}

We tend to keep the internal API calls first in the *.c files before the public API.

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

	/* verify the results */
	assert_int_equal(ret, 0);
	assert_ptr_equal(rcq, NULL);

You are missing a test case when rcq != NULL.

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

	/* verify the results */
	assert_int_equal(ret, 0);
	assert_ptr_equal(rcq, NULL);

I see you have assumed when the rcq_size == 0 the separate CQ for receives does not exist so the _get_rcq() will return NULL.
I am considering an alternative proposition when_get_rcq() returns a CQ for receives no matter if it is created separate or it is common with sends.
What do you think?

janekmi

Reviewed 15 of 59 files at r1, 16 of 17 files at r2.
Reviewable status: 38 of 59 files reviewed, 26 unresolved discussions (waiting on @yangx-jy)

tests/unit/common/mocks-rpma-conn.c, line 27 at r2 (raw file):

	check_expected_ptr(id);
	assert_ptr_equal(cq, MOCK_RPMA_CQ);
	assert_true(rcq == MOCK_RPMA_CQ || rcq == NULL);

I believe such an assertion is too wide. This way you have no way to distinguish and validate both cases. You can use mock_type(struct rpma_cq *) or any other technique we have already used.

tests/unit/common/mocks-rpma-conn_cfg.h, line 16 at r2 (raw file):

#define MOCK_CONN_CFG_CUSTOM	(struct rpma_conn_cfg *)0xCF6C

#define MOCK_RCQ_SIZE_DEFAULT   0

tab

tests/unit/common/mocks-rpma-conn_cfg.h, line 17 at r2 (raw file):

#define MOCK_RCQ_SIZE_DEFAULT   0
#define MOCK_CQ_SIZE_DEFAULT	10

I want to keep the order: CQ, RCQ.

tests/unit/common/mocks-rpma-conn_cfg.h, line 22 at r2 (raw file):

#define MOCK_TIMEOUT_MS_CUSTOM	4034
#define MOCK_RCQ_SIZE_CUSTOM    12

== MOCK_RQ_SIZE_DEFAULT. Please reassign the values to make them unique across the board.

tests/unit/common/mocks-rpma-conn_cfg.h, line 23 at r2 (raw file):

#define MOCK_TIMEOUT_MS_CUSTOM	4034
#define MOCK_RCQ_SIZE_CUSTOM    12
#define MOCK_CQ_SIZE_CUSTOM	13

.

tests/unit/common/mocks-rpma-cq.c, line 91 at r2 (raw file):

	struct rpma_cq *cq = *cq_ptr;
	assert_true(cq == MOCK_RPMA_CQ || cq == MOCK_RPMA_RCQ || cq == NULL);

Too permissive. Please use e.g. mock_type().

tests/unit/common/mocks-rpma-cq.c, line 109 at r2 (raw file):

rpma_cq_get_ibv_cq(const struct rpma_cq *cq)
{
	assert_true(cq == MOCK_RPMA_CQ || cq == MOCK_RPMA_RCQ);

.

tests/unit/common/mocks-rpma-peer.c, line 22 at r2 (raw file):

int
rpma_peer_create_qp(struct rpma_peer *peer, struct rdma_cm_id *id,
		struct rpma_cq *cq, struct rpma_cq *rcq,

The validation is missing.

tests/unit/conn/conn-new.c, line 319 at r2 (raw file):

	expect_value(rdma_destroy_qp, id, MOCK_CM_ID);
	will_return(rpma_cq_delete, RPMA_E_PROVIDER);
	will_return(rpma_cq_delete, EAGAIN);

MOCK_ERRNO?

tests/unit/conn/conn-new.c, line 353 at r2 (raw file):

	will_return(rpma_cq_delete, MOCK_OK);
	will_return(rpma_cq_delete, RPMA_E_PROVIDER);
	will_return(rpma_cq_delete, EAGAIN);

MOCK_ERRNO?

tests/unit/conn/conn-new.c, line 387 at r2 (raw file):

	expect_value(rdma_destroy_qp, id, MOCK_CM_ID);
	will_return(rpma_cq_delete, RPMA_E_PROVIDER);
	will_return(rpma_cq_delete, EAGAIN); /* first error */

MOCK_ERRNO?

tests/unit/conn/conn-new.c, line 389 at r2 (raw file):

	will_return(rpma_cq_delete, EAGAIN); /* first error */
	will_return(rpma_cq_delete, RPMA_E_PROVIDER);
	will_return(rpma_cq_delete, EIO); /* second error */

MOCK_ERRNO2?

tests/unit/conn/conn-new.c, line 391 at r2 (raw file):

	will_return(rpma_cq_delete, EIO); /* second error */
	expect_value(rdma_destroy_id, id, MOCK_CM_ID);
	will_return(rdma_destroy_id, EIO); /* third error */

MOCK_ERRNO2?

tests/unit/conn_cfg/CMakeLists.txt, line 31 at r2 (raw file):

add_test_conn_cfg(new)
add_test_conn_cfg(rcq_size)
add_test_conn_cfg(rq_size)

I believe the alphabetical order is:

rq_size
rcq_size

tests/unit/conn_req/conn_req-common.c, line 22 at r2 (raw file):

prestate_init(struct conn_req_new_test_state *prestate,
		struct rpma_conn_cfg *cfg, int timeout_ms,
		uint32_t cq_size, uint32_t rcq_size)

cq_size should still be in the previous line.

tests/unit/cq/cq-new_delete.c, line 203 at r2 (raw file):

	/* run test */
	int ret = rpma_cq_delete(&cq);

You are missing the case when cq_ptr == NULL.

janekmi

Reviewed 1 of 59 files at r1.
Reviewable status: 39 of 59 files reviewed, 27 unresolved discussions (waiting on @yangx-jy)

tests/unit/peer/peer-create_qp.c, line 40 at r2 (raw file):

static void
setup_create_qp(struct rpma_cq *rcq)

Since setup/teardown are reserved for the cmocka steps I think the right word here is configure_create_qp.

janekmi

Reviewed 1 of 59 files at r1.
Reviewable status: 40 of 59 files reviewed, 22 unresolved discussions (waiting on @yangx-jy)

janekmi

Reviewed 4 of 59 files at r1.
Reviewable status: 44 of 59 files reviewed, 22 unresolved discussions (waiting on @yangx-jy)

janekmi

Reviewable status: 44 of 59 files reviewed, 23 unresolved discussions (waiting on @yangx-jy)

a discussion (no related file):
You should merge these two PRs:

rpma: implement separate receiving CQ
test: add and refactor unit tests for seperate receiving CQ

By modifying code without changing the tests you making bisection impossible.

janekmi

Reviewed 9 of 59 files at r1.
Reviewable status: 53 of 59 files reviewed, 33 unresolved discussions (waiting on @yangx-jy)

src/conn_req.c, line 204 at r2 (raw file):

		ret = rpma_cq_delete(&req->cq);
	else
		(void) rpma_cq_delete(&req->cq);

	int ret2;

	int ret = rpma_cq_delete(&req->rcq);
	if ((ret2 = rpma_cq_delete(&req->cq)) {
		if (!ret)
			ret = ret2;
	}

src/conn_req.c, line 239 at r2 (raw file):

		ret = rpma_cq_delete(&req->cq);
	else
		(void) rpma_cq_delete(&req->cq);

.

src/peer.c, line 112 at r2 (raw file):

	qp_init_attr.qp_context = NULL;
	qp_init_attr.send_cq = ibv_cq;
	qp_init_attr.recv_cq = ibv_rcq ? ibv_rcq : ibv_cq;

... = rcq ? rpma_cq_get_ibv_cq(rcq) : ibv_cq;

tests/unit/conn_req/conn_req-delete.c, line 77 at r2 (raw file):

rpma_cq_delete(&req->cq) return RPMA_E_PROVIDER whereas rdma_ack_cm_event() and rdma_reject() fail with EIO after rpma_cq_delete(&req->rcq) returned RPMA_E_PROVIDER (a fail with EAGAIN)

I know, a little bit complicated.

tests/unit/conn_req/conn_req-delete.c, line 106 at r2 (raw file):

	assert_null(cstate->req);
}

You are missing here two cases for the delete_via_reject:

when the second rpma_cq_delete() fail + all subsequent pass
when the second rpma_cq_delete() fail + all subsequent fail

tests/unit/conn_req/conn_req-delete.c, line 227 at r2 (raw file):

rpma_cq_delete(&req->rcq) returns RPMA_E_PROVIDER whereas rdma_destroy_id() fails with EIO after rpma_cq_delete(&req->cq) returned RPMA_E_PROVIDER (a fail with EAGAIN)

tests/unit/conn_req/conn_req-delete.c, line 253 at r2 (raw file):

	assert_int_equal(ret, RPMA_E_PROVIDER);
	assert_null(cstate->req);
}

You are missing here two cases for the delete_via_destroy:

when the second rpma_cq_delete() fail + all subsequent pass
when the second rpma_cq_delete() fail + all subsequent fail

tests/unit/conn_req/conn_req-delete.c, line 308 at r2 (raw file):

	prestate_init(&prestate_conn_cfg_default, MOCK_CONN_CFG_DEFAULT,
			RPMA_DEFAULT_TIMEOUT_MS, MOCK_CQ_SIZE_DEFAULT,
			MOCK_RCQ_SIZE_DEFAULT);

Having the default RCQ size will cause the receive CQ to be nonexistent, am I right?

tests/unit/conn_req/conn_req-delete.c, line 308 at r2 (raw file):

	prestate_init(&prestate_conn_cfg_default, MOCK_CONN_CFG_DEFAULT,
			RPMA_DEFAULT_TIMEOUT_MS, MOCK_CQ_SIZE_DEFAULT,
			MOCK_RCQ_SIZE_DEFAULT);

We have to think if we need all of these to run in two flavours:

without the RCQ
with the RCQ

tests/unit/conn_req/conn_req-new.c, line 386 at r2 (raw file):

	will_return(__wrap__test_malloc, ENOMEM); /* first error */
	expect_value(rdma_destroy_qp, id, &id);
	will_return(rpma_cq_delete, MOCK_OK);

Here the first call to rpma_cq_delete() should also fail.

janekmi

Reviewed 2 of 59 files at r1.
Reviewable status: 55 of 59 files reviewed, 35 unresolved discussions (waiting on @yangx-jy)

tests/unit/conn_req/conn_req-new.c, line 241 at r2 (raw file):

	expect_value(rpma_cq_new, cqe, cstate->get_cqe.cq_size);
	if (cstate->get_cqe.rcq_size) {
		will_return(rpma_cq_new, MOCK_RPMA_CQ);

You are missing the case when rpma_cq_new() for the receives fails.

tests/unit/conn_req/conn_req-new.c, line 432 at r2 (raw file):

	cmocka_unit_test(new__peer_create_qp_E_PROVIDER_EAGAIN),
	cmocka_unit_test(new__malloc_ENOMEM),
	cmocka_unit_test(new__malloc_ENOMEM_subsequent_EAGAIN),

I think you should make the default and the custom variants of all these three tests also. Just in case the error handling will be somehow messed up.

janekmi

Reviewed 1 of 59 files at r1.
Reviewable status: 56 of 59 files reviewed, 36 unresolved discussions (waiting on @yangx-jy)

tests/unit/conn/conn-get_cq_rcq.c, line 93 at r2 (raw file):

/*
 * get_cq__success -- happy day scenario

rcq

janekmi

Reviewed 1 of 17 files at r2.
Reviewable status: 57 of 59 files reviewed, 50 unresolved discussions (waiting on @yangx-jy)

src/conn.c, line 479 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

get the connection's send CQ

get the main connection's CQ?

src/include/librpma.h, line 1335 at r2 (raw file):

/** 3
 * rpma_conn_cfg_set_rcq_size - set receiving CQ size for the connection

set receive CQ size for the connection

src/include/librpma.h, line 1346 at r2 (raw file):

 *
 * DESCRIPTION
 * rpma_conn_cfg_set_rcq_size() sets the receiving CQ size for the connection.

Please see the rpma_conn_get_recq() for details about the receive CQ.

src/include/librpma.h, line 1346 at r2 (raw file):

 *
 * DESCRIPTION
 * rpma_conn_cfg_set_rcq_size() sets the receiving CQ size for the connection.

~~the~~ receive CQ

src/include/librpma.h, line 1372 at r2 (raw file):

 *      struct rpma_conn_cfg;
 *      int rpma_conn_cfg_get_rcq_size(const struct rpma_conn_cfg *cfg,
 *                      uint32_t *rcq_size);

tabs

src/include/librpma.h, line 1706 at r2 (raw file):

/** 3
 * rpma_conn_get_cq -- get a rpma_cq object from the connection

get the main connection's CQ

src/include/librpma.h, line 1718 at r2 (raw file):

 *
 * DESCRIPTION
 * rpma_conn_get_cq() gets a rpma_cq object from the connection.

... get the main connection's CQ. When the receive CQ is not present the main connection's CQ allows handling all completions. When the receive CQ is present the main connection's CQ allows handling all completions except rpma_recv() completions. Please see rpma_conn_get_rcq() for details about the receive CQ.

src/include/librpma.h, line 1737 at r2 (raw file):

/** 3
 * rpma_conn_get_rcq -- get a receiving rpma_cq object from the connection

get the receive CQ from the connection

src/include/librpma.h, line 1749 at r2 (raw file):

 *
 * DESCRIPTION
 * rpma_conn_get_rcq() gets a receiving rpma_cq object from the connection.

... the receive CQ from the connection.

src/include/librpma.h, line 1749 at r2 (raw file):

 *
 * DESCRIPTION
 * rpma_conn_get_rcq() gets a receiving rpma_cq object from the connection.

The receive CQ allows handling all receive completion within the connection. It allows separating receive completion processing path from any other completion. The receive CQ is created only if the receive CQ size in the provided connection configuration is greater than 0. When the receive CQ does not exist for a given connection the *rcq_ptr == NULL.

src/include/librpma.h, line 1762 at r2 (raw file):

 * SEE ALSO
 * rpma_conn_req_connect(3), rpma_conn_get_cq(3), rpma_cq_wait(3),
 * rpma_cq_get_completion(3), rpma_cq_get_fd(3), librpma(7) and

rpma_conn_cfg_set_rcq_size(3)

src/include/librpma.h, line 2716 at r2 (raw file):

from CQ.

I do not like 'the rpma_cq object' because it conveys no meaning. Do you think we can refer to it directly as CQ?

@grom72 @yangx-jy

src/include/librpma.h, line 2716 at r2 (raw file):

 * DESCRIPTION
 * rpma_cq_get_fd() gets a file descriptor of the completion event channel
 * from the rpma_cq object.

This documentation is way too short to inform the end-user what they can do with this file descriptor. I think it deserves at least a new issue so we can handle this issue in the future.

src/include/librpma.h, line 2796 at r2 (raw file):

 * - RPMA_OP_RECV - messaging receive operation
 * - RPMA_OP_RECV_RDMA_WITH_IMM - messaging receive operation for
 *   RMA write operation with immediate data

Note that if the provided cq is the main CQ and the receive CQ is present this function won't return at any time RPMA_OP_RECV and RPMA_OP_RECV_RDMA_WITH_IMM. To collect these completions the receive CQ has to be used instead.

src/include/librpma.h, line 2814 at r2 (raw file):

 * rpma_conn_get_cq(3), rpma_conn_get_rcq(3), rpma_cq_wait(3),
 * rpma_cq_get_fd(3), rpma_flush(3), rpma_read(3), rpma_recv(3),
 * rpma_send(3), rpma_write(3), rpma_write_atomic(3), librpma(7)

You are missing the immediate variants here.

janekmi

Reviewed 2 of 59 files at r1.
Reviewable status: all files reviewed, 53 unresolved discussions (waiting on @yangx-jy)

a discussion (no related file):
I like this example so much. 😁

examples/09-flush-to-persistent-GPSPM/client.c, line 189 at r2 (raw file):

		goto err_free;

	if ((ret = rpma_conn_cfg_set_rcq_size(cfg, 10)))

You can add a define in the flush-to-persistent-GPSPM.h file:

/* only a single receive completion is expected */
#define RCQ_SIZE 1

examples/09-flush-to-persistent-GPSPM/server.c, line 202 at r2 (raw file):

		goto err_mr_dereg;

	if ((ret = rpma_conn_cfg_set_rcq_size(cfg, 10)))

10 -> RCQ_SIZE

examples/09-flush-to-persistent-GPSPM/server.c, line 248 at r2 (raw file):

		goto err_conn_delete;

	/* validate the receiving completion */

@grom72 do you know how completion of the RECV operation is referred to in the RDMA documentation? It is called a receive completion or a receiving completion? Any thoughts?

yangx-jy

Reviewable status: all files reviewed, 53 unresolved discussions (waiting on @grom72, @janekmi, and @yangx-jy)

examples/09-flush-to-persistent-GPSPM/server.c, line 248 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

@grom72 do you know how completion of the RECV operation is referred to in the RDMA documentation? It is called a receive completion or a receiving completion? Any thoughts?

It seems that only receive completion and receive CQ appear on IB Specification, so I perfer to replace receiving with receive.

src/conn.c, line 479 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

get the main connection's CQ?

get the main connection's CQ? or get the connection's main CQ?

src/conn_req.c, line 204 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

	int ret2;

	int ret = rpma_cq_delete(&req->rcq);
	if ((ret2 = rpma_cq_delete(&req->cq)) {
		if (!ret)
			ret = ret2;
	}

How about

int ret = rpma_cq_delete(&req->rcq);
int ret2 = rpma_cq_delete(&req->cq);

if (!ret && ret2)
	ret = ret2;

src/peer.c, line 112 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

... = rcq ? rpma_cq_get_ibv_cq(rcq) : ibv_cq;

Good idea.

src/include/librpma.h, line 1346 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

~~the~~ receive CQ

Other APIs all use the in DESCRIPTION.

src/include/librpma.h, line 2716 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

from CQ.

I do not like 'the rpma_cq object' because it conveys no meaning. Do you think we can refer to it directly as CQ?

@grom72 @yangx-jy

It is fine to me.

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I see you have assumed when the rcq_size == 0 the separate CQ for receives does not exist so the _get_rcq() will return NULL.
I am considering an alternative proposition when_get_rcq() returns a CQ for receives no matter if it is created separate or it is common with sends.
What do you think?

I perfer the current implementation.
It is easy for applications to know if separate receive CQ is used by _get_rcq (non-NULL or NULL).

tests/unit/conn/conn-new.c, line 319 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

MOCK_ERRNO?

I want to make this PR clear and simple so I don't apply simulated errno in this PR.
I have applied simulated errno for all unit tests in separate #1093. Perhaps, we needs to review it first.

yangx-jy

Reviewable status: 29 of 63 files reviewed, 53 unresolved discussions (waiting on @grom72, @janekmi, @ldorau, and @osalyk)

a discussion (no related file):

Previously, janekmi (Jan Michalski) wrote…

You should merge these two PRs:

rpma: implement separate receiving CQ

test: add and refactor unit tests for seperate receiving CQ

By modifying code without changing the tests you making bisection impossible.

Done.

examples/09-flush-to-persistent-GPSPM/client.c, line 189 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You can add a define in the flush-to-persistent-GPSPM.h file:
/* only a single receive completion is expected */
#define RCQ_SIZE 1

Done.

examples/09-flush-to-persistent-GPSPM/server.c, line 202 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

10 -> RCQ_SIZE

Done.

src/conn.c, line 258 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

cq first before rcq.

No, keep it.

src/conn.c, line 479 at r1 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

get the main connection's CQ? or get the connection's main CQ?

Done.

Use get the connection's main CQ.

src/conn.c, line 493 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

get the connection's receive CQ

Done.

src/conn_cfg.c, line 62 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

/*
 * rpma_conn_cfg... -- ...

Done.

Use ROUND_DOWN() macro in src/common.h instead.

src/conn_cfg.c, line 65 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

static int
rpma_conn_cfg_...

Done.

Use ROUND_DOWN() macro in src/common.h instead.

src/conn_cfg.c, line 71 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

It is not on the internal API of this unit so it should go above the internal API boundary.

TBH I think it may be an even better fit into common.h as a simple macro? Up to you.

Done.

Use ROUND_DOWN() macro in src/common.h instead.

src/conn_cfg.c, line 78 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I know it is tiresome to have getters and setters for each of the fields separately but I believe it makes the API easier to maintain in the future. The rpma_conn_cfg_get_cqe() function is a variant of rpma_conn_cfg_get_cq_size() and I think it should stay this way. Instead, I think you should introduce something like rpma_conn_cfg_get_rcqe() (following the naming conventions).

Done.

Add rpma_conn_cfg_get_rcqe()

src/conn_req.c, line 204 at r2 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

How about

int ret = rpma_cq_delete(&req->rcq);
int ret2 = rpma_cq_delete(&req->cq);

if (!ret && ret2)
	ret = ret2;

Done.

Use my above logic instead.

src/conn_req.c, line 239 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

Use my above logic instead.

src/cq.c, line 165 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

We tend to keep the internal API calls first in the *.c files before the public API.

Done.

src/include/librpma.h, line 1335 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

set receive CQ size for the connection

Done.

src/include/librpma.h, line 1346 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Please see the rpma_conn_get_recq() for details about the receive CQ.

Done.

src/include/librpma.h, line 1346 at r2 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Other APIs all use the in DESCRIPTION.

Keep the for now.

src/include/librpma.h, line 1372 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

tabs

Done.

src/include/librpma.h, line 1706 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

get the main connection's CQ

Done.

src/include/librpma.h, line 1718 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

... get the main connection's CQ. When the receive CQ is not present the main connection's CQ allows handling all completions. When the receive CQ is present the main connection's CQ allows handling all completions except rpma_recv() completions. Please see rpma_conn_get_rcq() for details about the receive CQ.

Done.

src/include/librpma.h, line 1737 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

get the receive CQ from the connection

Done.

get the connection's receive CQ

src/include/librpma.h, line 1749 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

... the receive CQ from the connection.

Done.

src/include/librpma.h, line 1749 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

The receive CQ allows handling all receive completion within the connection. It allows separating receive completion processing path from any other completion. The receive CQ is created only if the receive CQ size in the provided connection configuration is greater than 0. When the receive CQ does not exist for a given connection the *rcq_ptr == NULL.

Done.

src/include/librpma.h, line 1762 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

rpma_conn_cfg_set_rcq_size(3)

Done.

src/include/librpma.h, line 2716 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

This documentation is way too short to inform the end-user what they can do with this file descriptor. I think it deserves at least a new issue so we can handle this issue in the future.

Agreed, I will add a new issue about it.

src/include/librpma.h, line 2796 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Note that if the provided cq is the main CQ and the receive CQ is present this function won't return at any time RPMA_OP_RECV and RPMA_OP_RECV_RDMA_WITH_IMM. To collect these completions the receive CQ has to be used instead.

Done.

src/include/librpma.h, line 2814 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You are missing the immediate variants here.

Done.

tests/unit/common/mocks-rpma-conn.c, line 27 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I believe such an assertion is too wide. This way you have no way to distinguish and validate both cases. You can use mock_type(struct rpma_cq *) or any other technique we have already used.

Done.

Use check_expected_ptr() instead.

tests/unit/common/mocks-rpma-conn_cfg.h, line 16 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

tab

Done.

tests/unit/common/mocks-rpma-conn_cfg.h, line 17 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I want to keep the order: CQ, RCQ.

Done.

tests/unit/common/mocks-rpma-conn_cfg.h, line 22 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

== MOCK_RQ_SIZE_DEFAULT. Please reassign the values to make them unique across the board.

Done.

tests/unit/common/mocks-rpma-conn_cfg.h, line 23 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

tests/unit/common/mocks-rpma-cq.c, line 91 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Too permissive. Please use e.g. mock_type().

Done.

Use check_expected_ptr() instead.

tests/unit/common/mocks-rpma-cq.c, line 109 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

Use check_expected_ptr() instead.

tests/unit/common/mocks-rpma-peer.c, line 22 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

The validation is missing.

Done.

Add check_expected_ptr().

tests/unit/conn/conn-get_cq_rcq.c, line 93 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

rcq

Done.

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You are missing a test case when rcq != NULL.

Done.

get_rcq_without_rcq and get_rcq_with_rcq

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

I perfer the current implementation.
It is easy for applications to know if separate receive CQ is used by _get_rcq (non-NULL or NULL).

@grom72 @ldorau @osalyk @janekmi

We have to disscuss which implementation should be selected.

tests/unit/conn_cfg/CMakeLists.txt, line 31 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I believe the alphabetical order is:

rq_size

rcq_size

No. The correct one is rcq_size, rq_szie. See the output of sort:

# grep '^add_test_conn_cfg' CMakeLists.txt | sort -d
add_test_conn_cfg(cqe)
add_test_conn_cfg(cq_size)
add_test_conn_cfg(delete)
add_test_conn_cfg(new)
add_test_conn_cfg(rcqe)
add_test_conn_cfg(rcq_size)
add_test_conn_cfg(rq_size)
add_test_conn_cfg(sq_size)
add_test_conn_cfg(timeout)

tests/unit/conn_req/conn_req-common.c, line 22 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

cq_size should still be in the previous line.

Done.

I have removed prestate_init().

tests/unit/conn_req/conn_req-delete.c, line 77 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

rpma_cq_delete(&req->cq) return RPMA_E_PROVIDER whereas rdma_ack_cm_event() and rdma_reject() fail with EIO after rpma_cq_delete(&req->rcq) returned RPMA_E_PROVIDER (a fail with EAGAIN)

I know, a little bit complicated.

Done.

rpma_cq_delete(&req->rcq) fails with MOCK_ERRNO whereas subsequent (rpma_cq_delete(&req->cq), rdma_reject(), rdma_ack_cm_event())
fail with MOCK_ERRNO2

tests/unit/conn_req/conn_req-delete.c, line 106 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You are missing here two cases for the delete_via_reject:

when the second rpma_cq_delete() fail + all subsequent pass

when the second rpma_cq_delete() fail + all subsequent fail

Done.

delete_via_reject__rcq_delete_ERRNO()
delete_via_reject__rcq_delete_ERRNO_subsequent_ERRNO2()
delete_via_reject__cq_delete_ERRNO()
delete_via_reject__cq_delete_ERRNO_subsequent_ERRNO2()

tests/unit/conn_req/conn_req-delete.c, line 227 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

rpma_cq_delete(&req->rcq) returns RPMA_E_PROVIDER whereas rdma_destroy_id() fails with EIO after rpma_cq_delete(&req->cq) returned RPMA_E_PROVIDER (a fail with EAGAIN)

Done.

rpma_cq_delete(&req->rcq) fails with MOCK_ERRNO whereas subsequent (rpma_cq_delete(&req->cq), rdma_destroy_id()) fail with MOCK_ERRNO2

tests/unit/conn_req/conn_req-delete.c, line 253 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You are missing here two cases for the delete_via_destroy:

when the second rpma_cq_delete() fail + all subsequent pass

when the second rpma_cq_delete() fail + all subsequent fail

Done.

delete_via_destroy__rcq_delete_ERRNO()
delete_via_destroy__rcq_delete_ERRNO_subsequent_ERRNO2()
delete_via_destroy__cq_delete_ERRNO()
delete_via_destroy__cq_delete_ERRNO_subsequent_ERRNO2()

tests/unit/conn_req/conn_req-delete.c, line 308 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Having the default RCQ size will cause the receive CQ to be nonexistent, am I right?

Done.

Right.
Now, I use four structures (Conn_req_new_conn_cfg_default, Conn_req_new_conn_cfg_custom, Conn_req_conn_cfg_default, Conn_req_conn_cfg_custom) in all conn_req unit tests.

tests/unit/conn_req/conn_req-delete.c, line 308 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

We have to think if we need all of these to run in two flavours:

without the RCQ

with the RCQ

Done.

See the following structure:

static const struct CMUnitTest test_delete[] = {
...
}

tests/unit/conn_req/conn_req-new.c, line 241 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You are missing the case when rpma_cq_new() for the receives fails.

Done.

Add new__rcq_new_ERRNO()

tests/unit/conn_req/conn_req-new.c, line 386 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Here the first call to rpma_cq_delete() should also fail.

Done.

Only make rpma_cq_delete(&rcq) fail when rcq is not NULL.

tests/unit/conn_req/conn_req-new.c, line 432 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I think you should make the default and the custom variants of all these three tests also. Just in case the error handling will be somehow messed up.

Done.

See the following structure:

static const struct CMUnitTest test_new[] = {
...
}

tests/unit/cq/cq-new_delete.c, line 203 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

You are missing the case when cq_ptr == NULL.

No. This is an internel API.

ASSUMPTIONS
- cq_ptr != NULL

tests/unit/peer/peer-create_qp.c, line 40 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Since setup/teardown are reserved for the cmocka steps I think the right word here is configure_create_qp.

Done.

ldorau

Reviewed 3 of 59 files at r1, 3 of 34 files at r3, 1 of 1 files at r4.
Reviewable status: 33 of 63 files reviewed, 53 unresolved discussions (waiting on @grom72, @janekmi, and @osalyk)

tests/unit/conn_cfg/CMakeLists.txt, line 31 at r2 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

No. The correct one is rcq_size, rq_szie. See the output of sort:

# grep '^add_test_conn_cfg' CMakeLists.txt | sort -d
add_test_conn_cfg(cqe)
add_test_conn_cfg(cq_size)
add_test_conn_cfg(delete)
add_test_conn_cfg(new)
add_test_conn_cfg(rcqe)
add_test_conn_cfg(rcq_size)
add_test_conn_cfg(rq_size)
add_test_conn_cfg(sq_size)
add_test_conn_cfg(timeout)

It is OK

ldorau

Reviewed 1 of 59 files at r1, 2 of 34 files at r3.
Reviewable status: 35 of 63 files reviewed, 53 unresolved discussions (waiting on @grom72, @janekmi, and @osalyk)

janekmi

Reviewed 8 of 34 files at r3, 1 of 1 files at r4.
Reviewable status: 42 of 63 files reviewed, 19 unresolved discussions (waiting on @grom72, @janekmi, @osalyk, and @yangx-jy)

src/conn.c, line 479 at r1 (raw file):

Use 'get the connection's main CQ'.

I do agree.

src/include/librpma.h, line 1706 at r2 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Done.

After reconsideration, I think the connection's main CQ is correct. Please apply.

src/include/librpma.h, line 2716 at r2 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Agreed, I will add a new issue about it.

Please add a reference when it will be ready.

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

@grom72 @ldorau @osalyk @janekmi

We have to disscuss which implementation should be selected.

I like your argument. I think I am for returning NULL when the receive CQ is missing.
But I will just leave the issue to keep track of important API decisions I have to discuss with @grom72.

tests/unit/conn_cfg/CMakeLists.txt, line 31 at r2 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

It is OK

It must have been very late when I have written this nonsense. xD Sorry. Of course, you are right.

janekmi

Reviewed 1 of 34 files at r3.
Reviewable status: 43 of 63 files reviewed, 5 unresolved discussions (waiting on @grom72, @janekmi, @osalyk, and @yangx-jy)

src/conn.c, line 258 at r1 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

No, keep it.

I know these two are not created in a strict order but, since we have agreed so far to delete in the reverse order to what the items have been created, I will ask you one more time to switch these to in places. Please. 😁

yangx-jy

Reviewable status: 42 of 63 files reviewed, 5 unresolved discussions (waiting on @grom72, @janekmi, and @osalyk)

src/conn.c, line 258 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I know these two are not created in a strict order but, since we have agreed so far to delete in the reverse order to what the items have been created, I will ask you one more time to switch these to in places. Please. 😁

Hi Jan,

I checked all code and I think these two CQs in all place are deleted in the reverse order (i.e. rcq -> cq).
Did I miss anything?

src/conn.c, line 479 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Use 'get the connection's main CQ'.

I do agree.

Done.

src/include/librpma.h, line 1706 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

After reconsideration, I think the connection's main CQ is correct. Please apply.

Done.

src/include/librpma.h, line 2716 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Please add a reference when it will be ready.

#1112

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I like your argument. I think I am for returning NULL when the receive CQ is missing.
But I will just leave the issue to keep track of important API decisions I have to discuss with @grom72.

Sure, we should discuss it with @grom72.

By the way, I have rebased this PR. ^_^

yangx-jy

Reviewable status: 53 of 63 files reviewed, 13 unresolved discussions (waiting on @grom72, @janekmi, @ldorau, and @osalyk)

a discussion (no related file):

Previously, ldorau (Lukasz Dorau) wrote…

Please rebase - there is a merge conflict.

Done.

tests/unit/conn/conn-common.h, line 21 at r10 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Do setup_func or teardown_func can be anything else than NULL?

Done. Define four macros as you suggested.

tests/unit/conn_req/conn_req-common.h, line 24 at r10 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Can setup_func or teardown_func be anything else than NULL?

Done.

tests/unit/conn_req/conn_req-common.h, line 34 at r10 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I prefer having four macros (where two of them are really small) than having NULL, NULL all over the place. ;-)

Done.

tests/unit/conn_req/conn_req-from_cm_event.c, line 362 at r10 (raw file):

Previously, janekmi (Jan Michalski) wrote…

A new line is missing.

Done.

tests/unit/conn_req/conn_req-new.c, line 161 at r9 (raw file):

Previously, janekmi (Jan Michalski) wrote…

A subsequent fail test case is missing.

Done.

tests/unit/conn_req/conn_req-new.c, line 196 at r9 (raw file):

Previously, janekmi (Jan Michalski) wrote…

A subsequent fail test case is missing.

Done.

tests/unit/conn_req/conn_req-new.c, line 214 at r9 (raw file):

Previously, janekmi (Jan Michalski) wrote…

👍

Done.

tests/unit/conn_req/conn_req-new.c, line 237 at r9 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Sorry, only unit tests about resolve_addr and resolve_route are not related to seperate receive CQ.

Done. Added them in the PR.

tests/unit/conn_req/conn_req-new.c, line 282 at r9 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

tests/unit/conn_req/conn_req-new.c, line 335 at r9 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

tests/unit/conn_req/conn_req-new.c, line 443 at r9 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

? I think the comment has been updated.

ldorau

Reviewed 10 of 10 files at r11.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @grom72, @janekmi, and @osalyk)

janekmi

Reviewed 10 of 10 files at r11.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)

tests/unit/conn_req/conn_req-common.c, line 70 at r9 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Where do you want to define/set it?

After rethinking my approach does not make much sense. It is fine.

janekmi

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)

ldorau

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)

ldorau

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)

grom72

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @osalyk and @yangx-jy)

src/include/librpma.h, line 1720 at r11 (raw file):

rpma_conn_get_cq

completion queue has two meanings:

main completion queue of connection
any completion queue (main or recv)
Shall we distinguish this in the API names?
rpma_conn_get_main_cq?

src/include/librpma.h, line 1723 at r11 (raw file):

rpma_recv()

please harmonize this with rpma_conn_get_rcq description, where rpma_recv() is not mentioned.

src/include/librpma.h, line 2691 at r11 (raw file):

ERRORS

shall we refer here to rpma_cq_get_completion errors - instead of repeating errors values?

src/include/librpma.h, line 2786 at r11 (raw file):

Quoted 8 lines of code…

 *	enum rpma_op {
 *		RPMA_OP_READ,
 *		RPMA_OP_WRITE,
 *		RPMA_OP_FLUSH,
 *		RPMA_OP_SEND,
 *		RPMA_OP_RECV,
 *		RPMA_OP_RECV_RDMA_WITH_IMM,
 *	};

do we need this to be documented twice?

yangx-jy · 2021-08-24T06:25:51Z

src/include/librpma.h, line 1720 at r11 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

rpma_conn_get_cq
completion queue has two meanings:

main completion queue of connection

any completion queue (main or recv)
Shall we distinguish this in the API names?
rpma_conn_get_main_cq?

@grom72 @janekmi @ldorau @osalyk

Is the following design clear and more readable?
Change rpma_conn_get_cq to rpma_conn_get_main_cq
Change rpma_conn_get_rcq to rpma_conn_get_recv_cq

yangx-jy

Reviewable status: 58 of 63 files reviewed, 5 unresolved discussions (waiting on @grom72, @janekmi, @ldorau, and @osalyk)

src/include/librpma.h, line 1720 at r11 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

@grom72 @janekmi @ldorau @osalyk

Is the following design clear and more readable?
Change rpma_conn_get_cq to rpma_conn_get_main_cq
Change rpma_conn_get_rcq to rpma_conn_get_recv_cq

When we reach an agreement, I will change the name of API. (kepp it for now)

src/include/librpma.h, line 1723 at r11 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

rpma_recv()
please harmonize this with rpma_conn_get_rcq description, where rpma_recv() is not mentioned.

Done.

src/include/librpma.h, line 2691 at r11 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

ERRORS
shall we refer here to rpma_cq_get_completion errors - instead of repeating errors values?

Done.

src/include/librpma.h, line 2786 at r11 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

 *	enum rpma_op {
 *		RPMA_OP_READ,
 *		RPMA_OP_WRITE,
 *		RPMA_OP_FLUSH,
 *		RPMA_OP_SEND,
 *		RPMA_OP_RECV,
 *		RPMA_OP_RECV_RDMA_WITH_IMM,
 *	};

do we need this to be documented twice?

Done. Remove the same lines in rpma_conn_completion_get.3

grom72

Reviewable status: 58 of 63 files reviewed, 1 unresolved discussion (waiting on @janekmi, @ldorau, and @osalyk)

src/include/librpma.h, line 1720 at r11 (raw file):

Change rpma_conn_get_cq to rpma_conn_get_main_cq
Change rpma_conn_get_rcq to rpma_conn_get_recv_cq
Let's keep old names.

grom72

Reviewable status: 58 of 63 files reviewed, 1 unresolved discussion (waiting on @janekmi, @ldorau, and @osalyk)

a discussion (no related file):
We will mark "old" connection API as deprecated in a separate PR and will remove all conn_completion related calls in the following release.

grom72

Reviewable status: 58 of 63 files reviewed, 1 unresolved discussion (waiting on @janekmi, @ldorau, and @osalyk)

grom72

Reviewed 8 of 59 files at r1, 8 of 34 files at r3, 1 of 1 files at r4, 2 of 14 files at r6, 2 of 11 files at r7, 1 of 9 files at r9, 4 of 10 files at r11, 5 of 5 files at r12.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Sure, we should discuss it with @grom72.

By the way, I have rebased this PR. ^_^

+1 to return null

grom72

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)

tests/unit/conn/conn-get_cq_rcq.c, line 106 at r2 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

+1 to return null

OK

janekmi

Reviewed 5 of 5 files at r12.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @yangx-jy)

src/include/librpma.h, line 1723 at r11 (raw file):

Previously, yangx-jy (Xiao Yang) wrote…

Done.

receive completion is a not defined term. I think it will be easier to follow as rpma_recv(3).
Please also add to the SEE ALSO section.

src/include/librpma.h, line 1756 at r12 (raw file):

 * DESCRIPTION
 * rpma_conn_get_rcq() gets the receive CQ from the connection. The receive
 * CQ allows handling all receive completion within the connection. It allows

rpma_recv(3) completions + SEE ALSO

Note: Plural.

src/include/librpma.h, line 1757 at r12 (raw file):

 * rpma_conn_get_rcq() gets the receive CQ from the connection. The receive
 * CQ allows handling all receive completion within the connection. It allows
 * separating receive completion processing path from any other completion.

rpma_recv(3) completions

src/include/librpma.h, line 1757 at r12 (raw file):

 * rpma_conn_get_rcq() gets the receive CQ from the connection. The receive
 * CQ allows handling all receive completion within the connection. It allows
 * separating receive completion processing path from any other completion.

from all other completions.

src/include/librpma.h, line 2785 at r12 (raw file):

 * an already posted operation. All operations generate completion on error.
 * The operations posted with the RPMA_F_COMPLETION_ALWAYS flag also
 * generate a completion on success.  The following operations are available:

One space too much before The following....

1) add rpma_conn_cfg_set/get_rcq_size() and rpma_conn_cfg_get_rcqe() 2) add rpma_conn_get_cq/rcq() 3) public rpma_cq_wait/get_fd/get_completion() 4) add and refactor unit tests for seperate receive CQ Signed-off-by: Xiao Yang <[email protected]>

Signed-off-by: Xiao Yang <[email protected]>

janekmi

Reviewed 1 of 1 files at r13.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yangx-jy)

Ref: pmem#1080

Ref: pmem#1080 pmem#1220

Ref: pmem#1080

yangx-jy changed the title ~~implement separate recviving CQ~~ implement separate receiving CQ Jun 3, 2021

yangx-jy mentioned this pull request Jun 3, 2021

examples: make common functions accept conn_cfg #1067

Merged

yangx-jy force-pushed the separate_recv_cq branch from 07c8d2d to 7c524d9 Compare June 11, 2021 09:23

janekmi suggested changes Jun 11, 2021

View reviewed changes

janekmi reviewed Jun 11, 2021

View reviewed changes

janekmi mentioned this pull request Jun 11, 2021

Separate completion queue for receiving #737

Closed

janekmi changed the title ~~implement separate receiving CQ~~ rpma: implement separate receiving CQ Jun 11, 2021

janekmi suggested changes Jun 11, 2021

View reviewed changes

yangx-jy commented Jun 15, 2021

View reviewed changes

yangx-jy mentioned this pull request Jun 25, 2021

test: use simulated errno on all unit tests #1093

Merged

yangx-jy force-pushed the separate_recv_cq branch from 7c524d9 to ba2b96a Compare June 30, 2021 07:07

yangx-jy commented Jun 30, 2021

View reviewed changes

yangx-jy force-pushed the separate_recv_cq branch from ba2b96a to 9cacb3d Compare June 30, 2021 08:06

yangx-jy changed the title ~~rpma: implement separate receiving CQ~~ rpma: implement separate receive CQ Jun 30, 2021

yangx-jy requested a review from janekmi June 30, 2021 09:39

ldorau reviewed Jul 9, 2021

View reviewed changes

janekmi suggested changes Jul 22, 2021

View reviewed changes

yangx-jy force-pushed the separate_recv_cq branch from 9cacb3d to c02296e Compare July 23, 2021 02:17

yangx-jy commented Jul 23, 2021

View reviewed changes

yangx-jy commented Jul 27, 2021

View reviewed changes

ldorau reviewed Jul 27, 2021

View reviewed changes

janekmi reviewed Aug 6, 2021

View reviewed changes

janekmi approved these changes Aug 6, 2021

View reviewed changes

ldorau approved these changes Aug 16, 2021

View reviewed changes

ldorau approved these changes Aug 19, 2021

View reviewed changes

grom72 reviewed Aug 23, 2021

View reviewed changes

yangx-jy force-pushed the separate_recv_cq branch 2 times, most recently from f70cbd6 to 8c76ff9 Compare August 24, 2021 07:16

yangx-jy commented Aug 24, 2021

View reviewed changes

grom72 reviewed Aug 24, 2021

View reviewed changes

janekmi suggested changes Aug 24, 2021

View reviewed changes

yangx-jy force-pushed the separate_recv_cq branch from 8c76ff9 to 33951a4 Compare August 24, 2021 13:33

yangx-jy added 2 commits August 24, 2021 21:38

rpma: implement separate receive CQ

57a223d

1) add rpma_conn_cfg_set/get_rcq_size() and rpma_conn_cfg_get_rcqe() 2) add rpma_conn_get_cq/rcq() 3) public rpma_cq_wait/get_fd/get_completion() 4) add and refactor unit tests for seperate receive CQ Signed-off-by: Xiao Yang <[email protected]>

examples: make example-09 use seperate receive CQ

66b95b7

Signed-off-by: Xiao Yang <[email protected]>

yangx-jy force-pushed the separate_recv_cq branch from 33951a4 to 66b95b7 Compare August 24, 2021 13:38

janekmi mentioned this pull request Aug 24, 2021

Deprecate the rpma_conn_completion_*() functions #1212

Closed

janekmi approved these changes Aug 24, 2021

View reviewed changes

janekmi merged commit bfe7acb into pmem:master Aug 24, 2021

janekmi pushed a commit to janekmi/rpma that referenced this pull request Aug 24, 2021

doc: separate receive CQ

eff5cfa

Ref: pmem#1080

janekmi mentioned this pull request Aug 24, 2021

doc: separate receive CQ & rpma_mr_advise() #1216

Merged

janekmi pushed a commit to janekmi/rpma that referenced this pull request Sep 17, 2021

doc: separate receive CQ & rpma_mr_advise()

b6d2604

Ref: pmem#1080 pmem#1220

janekmi pushed a commit to janekmi/rpma that referenced this pull request Sep 17, 2021

rpma: implement separate receive CQ (fix)

21a8cdf

Ref: pmem#1080

janekmi mentioned this pull request Sep 17, 2021

rpma: implement separate receive CQ (fix) #1289

Merged

Patryk717 mentioned this pull request Mar 1, 2022

Do struct rpma_completion require rework? #976

Closed

rpma: implement separate receive CQ #1080

rpma: implement separate receive CQ #1080

Conversation

yangx-jy commented Jun 3, 2021 • edited by janekmi Loading

codecov bot commented Jun 3, 2021 • edited Loading

Codecov Report

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

yangx-jy left a comment

Choose a reason for hiding this comment

yangx-jy left a comment

Choose a reason for hiding this comment

ldorau left a comment

Choose a reason for hiding this comment

ldorau left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

yangx-jy left a comment

Choose a reason for hiding this comment

yangx-jy left a comment

Choose a reason for hiding this comment

ldorau left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

ldorau left a comment

Choose a reason for hiding this comment

ldorau left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

yangx-jy commented Aug 24, 2021

yangx-jy left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

yangx-jy commented Jun 3, 2021 •

edited by janekmi

Loading

codecov bot commented Jun 3, 2021 •

edited

Loading