From e508f8ebd50fd005e425cc95a2f80205502a4bbf Mon Sep 17 00:00:00 2001 From: Bruno Faccini Date: Tue, 23 Feb 2021 23:17:41 +0100 Subject: [PATCH 1/2] DAOS-6709 rsvc: xfer only attr size and fix tests Before this patch, when fetching pool/container attributes, if a segment buffer size is greater than attribute's real size, extra data is also leaked. This patch fixes this, and also modifies daos_test pool/container attributes related sub-tests to accomodate with the fact that when multiple attributes are requested, bulk transfer will cause their values to be packed within the different segments buffers. Change-Id: I989c384fdc667d12d8aeba3f67e51bb8be6ab290 Signed-off-by: Bruno Faccini --- src/rsvc/srv_common.c | 14 ++++++++++++-- src/tests/suite/daos_container.c | 9 ++++++--- src/tests/suite/daos_pool.c | 9 ++++++--- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/rsvc/srv_common.c b/src/rsvc/srv_common.c index dcab9949a64..362344489e7 100644 --- a/src/rsvc/srv_common.c +++ b/src/rsvc/srv_common.c @@ -243,8 +243,18 @@ ds_rsvc_get_attr(struct ds_rsvc *svc, struct rdb_tx *tx, rdb_path_t *path, svc->s_name, DP_KEY(&key), rc); goto out_iovs; } - iovs[j].iov_buf_len = sizes[i]; - sizes[i] = iovs[j].iov_len; + if (iovs[j].iov_len < sizes[i]) { + /* do not xfer more than attr length */ + bulk_size -= sizes[i] - iovs[j].iov_len; + /* return real size sent */ + sizes[i] = iovs[j].iov_len; + } else { + /* only return requested size */ + iovs[j].iov_buf_len = sizes[i]; + /* only attr size is requested */ + if (sizes[i] == 0) + sizes[i] = iovs[j].iov_len; + } /* If buffer length is zero, send only size */ if (iovs[j].iov_buf_len > 0) diff --git a/src/tests/suite/daos_container.c b/src/tests/suite/daos_container.c index e1f2c637705..c7a977a2a9e 100644 --- a/src/tests/suite/daos_container.c +++ b/src/tests/suite/daos_container.c @@ -110,6 +110,9 @@ co_attribute(void **state) }; int n = (int) ARRAY_SIZE(names); char out_buf[10 * BUFSIZE] = { 0 }; + /* use 2 consecutives pieces of big buffer to avoid the need to + * unpack and merge bulk segments + */ void *out_values[] = { &out_buf[0 * BUFSIZE], &out_buf[1 * BUFSIZE] @@ -169,12 +172,12 @@ co_attribute(void **state) print_message("Verifying Name-Value (A)..\n"); assert_int_equal(out_sizes[0], in_sizes[0]); - assert_memory_equal(out_values[0], in_values[0], in_sizes[0]); + assert_memory_equal(out_buf, in_values[0], in_sizes[0]); print_message("Verifying Name-Value (B)..\n"); assert_true(in_sizes[1] > BUFSIZE); - assert_int_equal(out_sizes[1], in_sizes[1]); - assert_memory_equal(out_values[1], in_values[1], BUFSIZE); + assert_int_equal(out_sizes[1], BUFSIZE); + assert_memory_equal(out_buf + out_sizes[0], in_values[1], out_sizes[1]); rc = daos_cont_get_attr(arg->coh, n, names, NULL, out_sizes, arg->async ? &ev : NULL); diff --git a/src/tests/suite/daos_pool.c b/src/tests/suite/daos_pool.c index 587dccac024..03c344804e8 100644 --- a/src/tests/suite/daos_pool.c +++ b/src/tests/suite/daos_pool.c @@ -245,6 +245,9 @@ pool_attribute(void **state) }; int n = (int) ARRAY_SIZE(names); char out_buf[10 * BUFSIZE] = { 0 }; + /* use 2 consecutives pieces of big buffer to avoid the need to + * unpack and merge bulk segments + */ void *out_values[] = { &out_buf[0 * BUFSIZE], &out_buf[1 * BUFSIZE] @@ -304,12 +307,12 @@ pool_attribute(void **state) print_message("Verifying Name-Value (A)..\n"); assert_int_equal(out_sizes[0], in_sizes[0]); - assert_memory_equal(out_values[0], in_values[0], in_sizes[0]); + assert_memory_equal(out_buf, in_values[0], in_sizes[0]); print_message("Verifying Name-Value (B)..\n"); assert_true(in_sizes[1] > BUFSIZE); - assert_int_equal(out_sizes[1], in_sizes[1]); - assert_memory_equal(out_values[1], in_values[1], BUFSIZE); + assert_int_equal(out_sizes[1], BUFSIZE); + assert_memory_equal(out_buf + out_sizes[0], in_values[1], out_sizes[1]); rc = daos_pool_get_attr(arg->pool.poh, n, names, NULL, out_sizes, arg->async ? &ev : NULL); From d9c7dca55cb463889aff22eb5e56d47a682ca326 Mon Sep 17 00:00:00 2001 From: Bruno Faccini Date: Wed, 17 Mar 2021 16:05:34 +0100 Subject: [PATCH 2/2] DAOS-6709 rsvc: send each segment as a single bulk This is a new way to fix extra data leak problem if destination buffer is larger than real attr size. To allow for only requested or attr exact size to be sent and avoid packing by Mercury, each segment has to be sent as a single bulk transfer instead as all being part of the same bulk transfer and have Mercury do own management. This allows to conform with original behaviour, so no associated tests need to be modified anymore. Change-Id: I02ea7227712a424fb8dd6b382c696fd1d4250db8 Signed-off-by: Bruno Faccini --- src/rsvc/srv_common.c | 38 +++++++++++++++++++++----------- src/tests/suite/daos_container.c | 9 +++----- src/tests/suite/daos_pool.c | 9 +++----- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/rsvc/srv_common.c b/src/rsvc/srv_common.c index 362344489e7..30faf17b1ba 100644 --- a/src/rsvc/srv_common.c +++ b/src/rsvc/srv_common.c @@ -176,6 +176,7 @@ ds_rsvc_get_attr(struct ds_rsvc *svc, struct rdb_tx *tx, rdb_path_t *path, crt_bulk_t local_bulk; daos_size_t bulk_size; daos_size_t input_size; + daos_size_t local_offset, remote_offset; d_iov_t *iovs; d_sg_list_t sgl; void *data; @@ -243,18 +244,8 @@ ds_rsvc_get_attr(struct ds_rsvc *svc, struct rdb_tx *tx, rdb_path_t *path, svc->s_name, DP_KEY(&key), rc); goto out_iovs; } - if (iovs[j].iov_len < sizes[i]) { - /* do not xfer more than attr length */ - bulk_size -= sizes[i] - iovs[j].iov_len; - /* return real size sent */ - sizes[i] = iovs[j].iov_len; - } else { - /* only return requested size */ - iovs[j].iov_buf_len = sizes[i]; - /* only attr size is requested */ - if (sizes[i] == 0) - sizes[i] = iovs[j].iov_len; - } + iovs[j].iov_buf_len = sizes[i]; + sizes[i] = iovs[j].iov_len; /* If buffer length is zero, send only size */ if (iovs[j].iov_buf_len > 0) @@ -269,7 +260,28 @@ ds_rsvc_get_attr(struct ds_rsvc *svc, struct rdb_tx *tx, rdb_path_t *path, goto out_iovs; rc = attr_bulk_transfer(rpc, CRT_BULK_PUT, local_bulk, remote_bulk, - 0, key_length, bulk_size - key_length); + 0, key_length, count * sizeof(*sizes)); + if (rc != 0) + goto out_iovs; + + local_offset = count * sizeof(*sizes); + remote_offset = key_length + count * sizeof(*sizes); + + for (i = 1; i < sgl.sg_nr; i++) { + daos_size_t size; + + size = min(sgl.sg_iovs[i].iov_len, + sgl.sg_iovs[i].iov_buf_len); + rc = attr_bulk_transfer(rpc, CRT_BULK_PUT, local_bulk, + remote_bulk, local_offset, + remote_offset, size); + if (rc != 0) + goto out_iovs; + + local_offset += sgl.sg_iovs[i].iov_buf_len; + remote_offset += sgl.sg_iovs[i].iov_buf_len; + } + crt_bulk_free(local_bulk); if (rc != 0) goto out_iovs; diff --git a/src/tests/suite/daos_container.c b/src/tests/suite/daos_container.c index c7a977a2a9e..e1f2c637705 100644 --- a/src/tests/suite/daos_container.c +++ b/src/tests/suite/daos_container.c @@ -110,9 +110,6 @@ co_attribute(void **state) }; int n = (int) ARRAY_SIZE(names); char out_buf[10 * BUFSIZE] = { 0 }; - /* use 2 consecutives pieces of big buffer to avoid the need to - * unpack and merge bulk segments - */ void *out_values[] = { &out_buf[0 * BUFSIZE], &out_buf[1 * BUFSIZE] @@ -172,12 +169,12 @@ co_attribute(void **state) print_message("Verifying Name-Value (A)..\n"); assert_int_equal(out_sizes[0], in_sizes[0]); - assert_memory_equal(out_buf, in_values[0], in_sizes[0]); + assert_memory_equal(out_values[0], in_values[0], in_sizes[0]); print_message("Verifying Name-Value (B)..\n"); assert_true(in_sizes[1] > BUFSIZE); - assert_int_equal(out_sizes[1], BUFSIZE); - assert_memory_equal(out_buf + out_sizes[0], in_values[1], out_sizes[1]); + assert_int_equal(out_sizes[1], in_sizes[1]); + assert_memory_equal(out_values[1], in_values[1], BUFSIZE); rc = daos_cont_get_attr(arg->coh, n, names, NULL, out_sizes, arg->async ? &ev : NULL); diff --git a/src/tests/suite/daos_pool.c b/src/tests/suite/daos_pool.c index 03c344804e8..587dccac024 100644 --- a/src/tests/suite/daos_pool.c +++ b/src/tests/suite/daos_pool.c @@ -245,9 +245,6 @@ pool_attribute(void **state) }; int n = (int) ARRAY_SIZE(names); char out_buf[10 * BUFSIZE] = { 0 }; - /* use 2 consecutives pieces of big buffer to avoid the need to - * unpack and merge bulk segments - */ void *out_values[] = { &out_buf[0 * BUFSIZE], &out_buf[1 * BUFSIZE] @@ -307,12 +304,12 @@ pool_attribute(void **state) print_message("Verifying Name-Value (A)..\n"); assert_int_equal(out_sizes[0], in_sizes[0]); - assert_memory_equal(out_buf, in_values[0], in_sizes[0]); + assert_memory_equal(out_values[0], in_values[0], in_sizes[0]); print_message("Verifying Name-Value (B)..\n"); assert_true(in_sizes[1] > BUFSIZE); - assert_int_equal(out_sizes[1], BUFSIZE); - assert_memory_equal(out_buf + out_sizes[0], in_values[1], out_sizes[1]); + assert_int_equal(out_sizes[1], in_sizes[1]); + assert_memory_equal(out_values[1], in_values[1], BUFSIZE); rc = daos_pool_get_attr(arg->pool.poh, n, names, NULL, out_sizes, arg->async ? &ev : NULL);