[knard38]

This patch is split off from the PR #18941 , which had grown too large to review as a single PR. It carries the hardening of the existing csum_dump path, while #18941 keeps the new dv_check_csum() API. Once this patch has landed, master will be merged into #18941 so that only the check part remains there.

The principle behind the change was raised during the review of #18941: ddb is a recovery tool, so the checksum metadata it reads from a possibly damaged pool must be neither trusted nor asserted on. dv_dump_csum() handed the checksum infos returned by VOS straight to its callback. The csum_dump callbacks of ddb_commands.c then asserted that the checksum-info count matches the extent count and that every checksum info is valid, and dereferenced the checksum type through daos_mhash_type2algo() without checking it. A pool with corrupted checksum metadata could therefore crash ddb instead of getting an error.

validate the fetched checksum metadata (ddb_vos.c, ddb_vos.h)

dump_csum_sv() and dump_csum_recx() now check what the VOS_OF_FETCH_CSUM fetch returned before invoking the callback:

1. Checksum-info count. A single value has zero or exactly one checksum info. For an array akey, the checksum-info list is either empty (akey without checksums: the stored extents are still reported) or has exactly one entry per entry of the recx list: the checksum configuration is stored at the root of the evtree, so evt_entry_csum_fill() reports a checksum info for every non-hole extent while akey_fetch_recx() records one recx per such extent. A count that does not match denotes corrupted metadata or a broken VOS invariant.
2. Checksum-info validity (csum_info_check()). Every entry must pass ci_is_valid() (sizes, count, buffer), and its checksum type must be known to the checksum library (daos_mhash_type2algo()), since the callbacks rely on that lookup to print the type name.
3. Chunk coverage (csum_recx_check_chunks()). The number of checksums of a recx entry must be the number of chunks of its extent (daos_recx_calc_chunks()). Since DAOS-19629 (DAOS-19629 vos: Do not trim checksums fetched with VOS_OF_FETCH_CSUM #19057) VOS no longer trims the checksums fetched with VOS_OF_FETCH_CSUM, so every recx-list entry is reported with its whole physical extent and its checksum info covers all of its chunks; a mismatch used to be printed silently as a Record Indexes line paired with too few checksum values.

Every inconsistency is logged with D_ERROR and returned as -DER_CSUM without invoking the callback, the way a data mismatch is reported. The callback contract is exhaustively documented in ddb_vos.h.

For the existing csum_dump callbacks of ddb_commands.c (untouched here), the D_ASSERT()s on the count and on ci_is_valid() and the daos_mhash_type2algo() dereference are now guaranteed by the API contract. On a pool with inconsistent checksum metadata, the ddb CLI reports DER_CSUM(-1032): Checksum error and the details go to the DAOS log, instead of an abort.

Fault injection (common.h)

Three new fail-locs in the DDB group make the error paths testable on a real pool, since VOS cannot produce such metadata: DDB_CSUM_NR_INJECT forges the checksum-info count, DDB_CSUM_CHUNK_NR_INJECT the chunk count of a checksum info and DDB_CSUM_TYPE_INJECT an unknown checksum type. The corresponding tests skip on builds without FAULT_INJECTION.

Test fixtures (tests/ddb_test_driver.c, tests/ddb_test_driver.h)

The checksum fixtures now name their objects and the data bytes they write with the DVT_FAKE_CSUM_OID_* and DVT_FAKE_PART_* constants, so that the tests refer to the fixture objects by role instead of by index. csum_test_partial_recx_setup() adds an object with an older extent A = [0, S)@1 whose leading chunk is overwritten by a newer extent B = [0, S/2)@2, and keeps their checksum infos in dct_part_ics so that the tests can check that A is reported with the checksums of both its visible and its hidden chunk.

Tests (tests/ddb_vos_tests.c, tests/ddb_commands_tests.c, tests/ddb_cmocka.h)

Test	What it covers
dump_csum_error_tests	Invalid pool handle → vos_cont_open failure propagation
dump_csum_sv_tests	SV path: no checksum; checksum at epoch 1 and at DAOS_EPOCH_MAX (latest version); no-callback no-op; callback-error propagation
dump_csum_recx_tests	Array path: no checksum; valid checksums paired with their extents; partially overwritten extent (the hidden chunk's checksum is reported too); range starting inside an extent (whole checksum info reported); range without any stored extent (callback invoked with recx_rel == NULL and an empty list); no-callback no-op; callback-error propagation
dump_csum_sv_inconsistent_tests (FI)	Two checksum infos for a single value; one claimed for a value without any (dcs_csum_info_get() returns NULL); unknown checksum type → -DER_CSUM, callback not invoked; control step with the consistent count
dump_csum_recx_inconsistent_tests (FI)	Fewer/more checksum infos than extents; infos claimed for extents without any; info claimed for a range without any stored extent; checksum covering more/fewer chunks than its extent; unknown checksum type → -DER_CSUM, callback not invoked; control steps with the consistent counts
print_csum_recx_tests, write_csum_recx_tests	csum_dump output for the partially overwritten fixture: one Record Indexes line per stored extent, in extent order, with every chunk checksum (printed_count(), assert_strings_in_order())

ddb_cmocka.h gains the assert_string_contains() and assert_strings_in_order() helpers used by the command tests.

Local Validation

• ddb_tests 76/76 on a fault-injection build, including the checksum cases of ddb_vos_tests and ddb_commands_tests, also clean under valgrind; ddb_ut 22/22.
• End to end on a real single-rank pool with a cksum:crc16 container and a container without checksums: 420 csum_dump invocations over every akey_sv_* and akey_array_* path of the pool, all exiting 0 with the expected crc16 values or No checksum at lines.

Steps for the author:

• Commit message follows the guidelines.
• Appropriate Features or Test-tag pragmas were used.
• Appropriate Functional Test Stages were run.
• At least two positive code reviews including at least one code owner from each category referenced in the PR.
• Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

• Gatekeeper requested (daos-gatekeeper added as a reviewer).

[github-actions]

Ticket title is 'Checksum management with ddb'
Status is 'In Progress'
Labels: 'ddb'
https://daosio.atlassian.net/browse/DAOS-17321

[daosbuild3]

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19177/2/execution/node/1892/log

[janekmi]

Have you considered simply mocking VOS interfaces and test just the DDB logic here? It looks like a lot of effort is actually put in testing the VOS logic here.

I understand if you decide to leave it as it is. In this PR you are only improving already existing tests. Re-writing all of this will be probably a significant effort but with a potential to also significantly simplify all of this test code.

[knard38]

Have you considered simply mocking VOS interfaces and test just the DDB logic here? It looks like a lot of effort is actually put in testing the VOS logic here.

I understand if you decide to leave it as it is. In this PR you are only improving already existing tests. Re-writing all of this will be probably a significant effort but with a potential to also significantly simplify all of this test code.

@janekmi thanks for the suggestion. I actually tried mocking the VOS interfaces while the work was still in the original PR #18941, before splitting it (short summary in #18941 (comment)). It worked, but needed a lot of plumbing, which is why I settled on fault injection.

In more detail: the count mismatch cannot be built through the VOS API (a "mixed" akey, with one extent written with a checksum and one without, still gets a checksum info for every extent), so I prototyped two mock variants: a single -Wl,--wrap=vos_ioh2ci in ddb_tests (which forced ddb_tests to link the ddb objects instead of libddb.so), and then a poolless harness in ddb_ut with a ddb_mocks.[hc] layer wrapping the seven VOS calls these functions make (vos_cont_open/close, vos_fetch_begin/end, vos_ioh2ci, vos_ioh2recx_list, vos_ioh2sv_epoch). Both worked, but the mock layer was several hundred lines for what were then four count checks, pinned the exact VOS call sequence of the functions and re-implemented the ownership rules of the returned lists by hand. Fault injection is a few DAOS_FAIL_CHECK hooks, the same approach VOS uses for corrupted metadata (vos_pool_check(), svt_rec_store()), and it runs on the real pool and cleanup path.

[knard38]

@janekmi , all your concerns should be fixed with the commit 176aa33

[knard38]

@janekmi your last concerns should be fixed with commit a3fdc9d

[daosbuild3]

Test stage Functional on EL 9 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19177/6/execution/node/1373/log

[daosbuild3]

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19177/8/execution/node/621/log

[daosbuild3]

Test stage Functional Hardware Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-19177/7/testReport/

[daosbuild3]

Test stage Functional Hardware Medium Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19177/9/execution/node/2028/log

[daosbuild3]

Test stage Functional on EL 9 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19177/10/execution/node/1372/log

[daosbuild3]

Test stage Functional Hardware Medium Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19177/10/execution/node/1876/log

[daosbuild3]

Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19177/10/execution/node/1819/log

[knard38]

@daos-stack/daos-gatekeeper could you lend this PR with the following message:

• title: DAOS-17321 ddb: Report inconsistent csum metadata in dump_csum
• body:*

ddb is a recovery tool, so it must neither trust nor assert on the
checksum metadata it reads from a possibly damaged pool.
dv_dump_csum() passed the fetched checksum infos unchecked to
callbacks that asserted on them, so a corrupted pool could crash ddb.

Validate the fetched metadata before invoking the callback: the
checksum-info list must be empty or have one entry per stored value
or extent, and every checksum info must pass ci_is_valid(), have a
type known to the checksum library and, for an array, cover exactly
the chunks of its extent. Any inconsistency is logged with its
object, dkey and akey and returned as -DER_CSUM, so ddb now reports
a checksum error instead of aborting.

Document the callback contract in ddb_vos.h, and add the
DDB_CSUM_NR_INJECT, DDB_CSUM_CHUNK_NR_INJECT and DDB_CSUM_TYPE_INJECT
fault injection points, with ddb_tests fixtures and cases for
dv_dump_csum() and the print and write csum_dump callbacks.