Skip to content

Commit

Permalink
DAOS-6709 rsvc: send each segment as a single bulk
Browse files Browse the repository at this point in the history
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 <bruno.faccini@intel.com>
  • Loading branch information
bfaccini committed Mar 17, 2021
1 parent af2534e commit d9c7dca
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 25 deletions.
38 changes: 25 additions & 13 deletions src/rsvc/srv_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down
9 changes: 3 additions & 6 deletions src/tests/suite/daos_container.c
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 3 additions & 6 deletions src/tests/suite/daos_pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit d9c7dca

Please sign in to comment.