Repository navigation
Perftest: Add synchronous data validation - #364
NitzanLavy wants to merge 6 commits into
Conversation
|
Hi @NitzanLavy, thanks for the effort and contribution. |
|
@sshaulnv It's unfortunate that a capability meant to be general was pushed without any community review and in a way that pins its availability only to RC mode and only to devices that support ATOMICs. |
|
@mrgolin The idea behind this approach was to stick to the 'performance-first' goal of perftest. We wanted a data validation that doesnt backpressure the requester, so the benchmark reflects true hardware performance even when validation is toggled on. Since we need both performance and generality, i suggest to support two modes: Asynchronous (Default: Synchronous ( |
|
Reworking this to coexist with the asynchronous data validation that has since merged, following the two-mode split @sshaulnv proposed above — async stays the default, It depends on the fixes in #407 and won't work until those land, so I'll push the update once it is merged. |
|
@NitzanLavy , 407 is merged now, please rebase and i will review and test. |
Adding a capability in perftest to validate the integrity of the data transferred. For now this works only for write_bw test scenarios. Perftest already provides a performance oriented data validation which verifies out of band and never backpressures the requestor, at the cost of requiring an RC connection and ATOMIC support. That leaves the capability unavailable on transports and devices which do not offer ATOMICs. This adds a second kind which validates inline on the receive path, works over any transport and any device, and throttles the sender in exchange. The two kinds are represented as one validation type rather than two independent flags, so they cannot be requested together, and the existing flag keeps its current meaning when no kind is given: --data_validation asynchronous, as before --data_validation=async asynchronous, explicitly --data_validation=sync synchronous The synchronous kind generates the payload from a seed and has the receiver regenerate an identical reference buffer to compare the incoming data against. Each kind needs to publish exactly one value to its peer, so both reuse the same field of the address exchange rather than adding a second one, and both publish it from the same place while setting up the connection. The exchange itself is left untouched. Data validation implementation has a few limitations. The following flags and conditions must be applied when running the synchronous kind: post_list (-l) == tx_depth (-t) recv_post_list (--recv_post_list) == rx_depth (-r) --write_with_imm host memory only Turning on the immediate flag is also required as the immediate field is used as a hint for the data. Reviewed-by: Firas Jahjah <firasj@amazon.com> Reviewed-by: Daniel Kranzdorf <dkkranzd@amazon.com> Signed-off-by: Nitzan Lavy <nlavy@amazon.com>
Extending synchronous data validation to offer a serial payload in addition to the random one. The data buffer of which the packets consists of contains serial data in granularity of DWORD. The fill is selected independently of the validation kind, since it only describes how the synchronous kind generates its payload: --data_validation=sync --data_validation_fill=serial The user can set the initial value from which the data will increment from by using the flag --data_start_value. By default the data will have a starting value of 0. As an example, if the user used the flag --data_start_value 64, it will result in the following data: 00000040 00000041 00000042 ... Reviewed-by: Firas Jahjah <firasj@amazon.com> Reviewed-by: Daniel Kranzdorf <dkkranzd@amazon.com> Signed-off-by: Nitzan Lavy <nlavy@amazon.com>
Extending synchronous data validation to offer a pattern payload in addition to the random and serial ones. The pattern is taken from an input file, using the flag --payload_file_path. The format of the pattern input file is DWORD in hex, followed by a comma. An example: 0xaaaaaaaa,0xbbbbbbbb,... The patterns will be used in cyclic way to fill the whole data buffer. --data_validation=sync --data_validation_fill=pattern \ --payload_file_path=<file> Reviewed-by: Firas Jahjah <firasj@amazon.com> Reviewed-by: Daniel Kranzdorf <dkkranzd@amazon.com> Signed-off-by: Nitzan Lavy <nlavy@amazon.com>
Extending synchronous data validation to send_bw in addition to write_bw. Random, serial and pattern payloads are all supported, as they are for write. Like in write scenarios, using the immediate field is required for data validation to work, since it carries the offset the receiver validates from. For send that means the following flag must be applied: --send_with_imm Unlike write, a send lands in whichever receive buffer the device consumed rather than at an address the sender chose, so the offset cannot locate it. The receiver recovers the buffer from the work request id instead. Reviewed-by: Firas Jahjah <firasj@amazon.com> Reviewed-by: Daniel Kranzdorf <dkkranzd@amazon.com> Signed-off-by: Nitzan Lavy <nlavy@amazon.com>
Create a file dump when data validation fails, to ease the debugging effort. The dump names the queue pair, the compared length and offset, the fill kind and the seed, and lists the DWORDs that differ. Only those DWORDs are dumped by default. Add --dump_full_buffers for the whole reference and received buffers, which under a size sweep can reach tens of megabytes and is not limited. Reviewed-by: Ramon Fried <friedram@amazon.com> Reviewed-by: Yonatan Nachum <ynachum@amazon.com> Signed-off-by: Nitzan Lavy <nlavy@amazon.com>
Extend synchronous data validation to UD. A UD receive places a 40 byte GRH ahead of the payload and counts it in the completion length, so the comparison skips the header and takes it off the length before comparing. Other transports are unaffected. Reviewed-by: Daniel Kranzdorf <dkkranzd@amazon.com> Reviewed-by: Yonatan Nachum <ynachum@amazon.com> Signed-off-by: Nitzan Lavy <nlavy@amazon.com>
1bfe596 to
a015731
Compare
|
Thank you @sshaulnv , I appreciate it. I rebased this data validation version now on updated master and updated the description of the PR to represent current situation. |
|
Hi @sshaulnv , just making sure you saw my last update that I rebased the code. Would appreciate your review. |
|
Hi @NitzanLavy , im aware of the rebase. sorry about the delay, will review it. |
|
@NitzanLavy
Same command for each size. Link MTU was 1024, so none of these are truncated. A pass prints only the bandwidth table. A failure prints Server: ib_send_bw -d rocep8s0f0 -x 3 -c UD -t 32 -l 32 -r 32 --recv_post_list=32 -Q 32 -n 32 -s 512 --data_validation=sync --send_with_immClient: ib_send_bw -d rocep8s0f0 -x 3 -c UD -t 32 -l 32 -r 32 --recv_post_list=32 -Q 32 -n 32 -s 512 --data_validation=sync --send_with_imm 192.168.200.4 |
|
|
||
| validation_fill_flag = 0; | ||
| } | ||
| if (data_validation_flag) { |
There was a problem hiding this comment.
A separate word is ignored. --data_validation sync leaves the argument empty and selects async. Together with --write_with_imm it then fails the async verb check.
|
|
||
| if (verify_imm_data && wc[i].imm_data != expected_imm_data) { | ||
| if (verify_imm_data && wc[i].imm_data != expected_imm_data && | ||
| !validation_is_sync(user_param->data_validation)) { |
There was a problem hiding this comment.
unless you are planning to add support for run_infinitely mode, i think its better to reject --run_infinitely with data validation.
| fprintf(dump_file, "QP Index: %d\n", qp_index); | ||
| fprintf(dump_file, "Data Length: %u bytes\n", data_length); | ||
| fprintf(dump_file, "Receive Offset: %u\n", recv_offset); | ||
| fprintf(dump_file, "Validation Type: %s\n", validationFillStr[user_param->validation_fill]); |
There was a problem hiding this comment.
The default fill is printed as none, while the generator treats that as random
Validation Type: none
| fprintf(dump_file, "Receive Offset: %u\n", recv_offset); | ||
| fprintf(dump_file, "Validation Type: %s\n", validationFillStr[user_param->validation_fill]); | ||
| if (user_param->validation_fill == VALIDATION_FILL_SERIAL) { | ||
| fprintf(dump_file, "Data Start Value: %u\n", user_param->data_start_value); |
There was a problem hiding this comment.
On the server this is still 0 when only the client passed --data_start_value. The hint on the next line is the value that was used.
Validation Type: serial
Data Start Value: 0
| COMPARE(compare, no_enhanced_reorder, INT, true, ((char *[]){"OFF", "ON"})), | ||
| COMPARE(compare, sig_offload, INT, true, ((char *[]){"OFF", "ON"})), | ||
| COMPARE(compare, data_validation, INT, true, ((char *[]){"OFF", "ON"})), | ||
| COMPARE(compare, data_validation, INT, true, ((char *[]){"NONE", "ASYNC", "SYNC"})), |
There was a problem hiding this comment.
The validation mode is negotiated, the fill is not.
Serial on one side and the default on the other fails the compare. The serial start can stay in the hint.
| } | ||
|
|
||
| if (validation_is_sync(user_param->data_validation)) { | ||
| if (user_param->post_list != user_param->tx_depth || user_param->recv_post_list != user_param->rx_depth) { |
There was a problem hiding this comment.
Sync validation is send-with-immediate or write-with-immediate, and both have a receive completion. For those, an odd receive depth is bumped by one before the new check runs. --recv_post_list is left as typed. So -r 31 --recv_post_list=31 becomes receive depth 32 and post list 31, and the equality check fails. An even value such as 32 is left alone and passes.
ib_send_bw -d rocep8s0f0 -x 3 -c UD -t 32 -l 31 -r 31 --recv_post_list=31 -Q 31 -n 31 -s 512 --data_validation=sync --send_with_imm --data_validation_fill=serial
WARNING: BW peak won't be measured in this run.
Disabling dynamic polling
---------------------------------------------------------------------------------------
Invalid data validation qps configuration. Post list size should be equal to corresponding queue depth.
| if (validation_is_async(user_param.data_validation)) { | ||
| sleep(2); | ||
| } | ||
| if (user_param.test_method == RUN_ALL) { |
There was a problem hiding this comment.
-a changes the size on every iteration, up to 8MB. The reference buffer and the stride were built once, from the size at startup, and this loop does not rebuild them.
Async already rejects -a. Sync should too, or it should rebuild the reference and the stride for each size. send_bw.c:385 has the same one-time reference, and its all-sizes loop does not rebuild it either.
| fprintf(dump_file, "Offset Expected Actual\n"); | ||
| fprintf(dump_file, "------ -------- ------\n"); | ||
|
|
||
| for (i = 0; i < data_length; i += 4) { |
There was a problem hiding this comment.
The 4-byte load runs past the end when the length is not a multiple of 4. The copy just above is allocated to the exact length.
Description
Upstream already has a performance-oriented data validation that verifies out of band and never backpressures the requestor. It costs an RC connection and ATOMIC support, which leaves the capability unavailable on transports and devices that do not offer ATOMICs — EFA among them, since it has no RC support at all.
This adds a second kind that validates inline on the receive path. It needs no ATOMICs or RC support. The two data validation types are modes of one option rather than competing flags, and the existing flag keeps its current meaning when no mode is given:
This is the split @sshaulnv proposed in this PR and it answers @mrgolin's point that async validation "pins its availability only to RC mode and only to devices that support ATOMICs".
The synchronous mode generates the payload from a seed and has the receiver regenerate an identical reference buffer to compare against. Three fill modes are available via
--data_validation_fill:random(default),serial(with--data_start_value), andpattern(from a file given with--payload_file_path). On a mismatch the receiver writes a dump of the mismatching DWORDs, or of both buffers in full with--dump_full_buffers.Each mode needs to publish exactly one value to its peer, so both reuse the same field of the address exchange rather than adding a second one, and both publish it from the same place while setting up the connection.
Scope
Supported: send BW and write BW, both with immediate. The immediate field carries the hint the receiver validates from, so
--send_with_imm/--write_with_immis required. Tested over SRD and UD.Constraints
Commits
serialfill,--data_start_valuepatternfill from--payload_file_path--dump_full_buffers