Skip to content

Commit 790b251

Browse files
GarthSnyderGarthSnyder
committed
Consistently encode DRR_BEGIN packed nvlist payloads with NV_ENCODE_XDR
This is a fix for #18360. Currently, zfs send generates a mix of nvlist encodings in DRR_BEGIN records, some XDR and some in native byte order. The result is that most streams currently can't be zfs received on opposite-endian systems. zfs send generates the outer wrappers for compound streams in userspace, and it explicitly requests NV_ENCODE_XDR format for those records. But the BEGIN records for individual datasets are generated on the kernel side, in dmu_send.c, where fnvlist_pack() is used for encoding. That routine hard-wires NV_ENCODE_NATIVE format. This PR replaces the fnvlist_pack() call with a direct call to nvlist_pack() that specifies NV_ENCODE_XDR. Tests are included to verify that native-encoded nvlists are not generated by any kernel path that attaches nvlists to BEGIN records. There's also a check for XDR encoding in the outer wrapper of replication streams in case there is ever a regression there. There are also two tests that have a chance of triggering (and detecting) bug #18491. Non-triggering versions of those tests are already included here, so when that bug is more fully characterized, the tests can be moved to a more directly relevant category. (They are the two tests with _with_write suffixes.) This PR adds to zstream dump an output line that shows the exact encoding of any nvlists in BEGIN records. This feature is used by the tests to validate streams. Signed-off-by: Garth Snyder <garth@garthsnyder.com>
1 parent 6a25950 commit 790b251

23 files changed

Lines changed: 1481 additions & 2 deletions

‎cmd/zstream/zstream_dump.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,20 @@ zstream_do_dump(int argc, char *argv[])
385385
(void) ssread(buf, sz, &zc);
386386
if (ferror(send_stream))
387387
perror("fread");
388+
389+
uint8_t *nv_header = (uint8_t *)buf;
390+
boolean_t xdr = nv_header[0] == NV_ENCODE_XDR;
391+
boolean_t big_endian = nv_header[1] == 0;
392+
const char *nc;
393+
if (xdr) {
394+
nc = "NV_ENCODE_XDR";
395+
} else if (big_endian) {
396+
nc = "NV_ENCODE_NATIVE (big-endian)";
397+
} else {
398+
nc = "NV_ENCODE_NATIVE (little-endian)";
399+
}
400+
printf("nvlist encoding = %s\n", nc);
401+
388402
err = nvlist_unpack(buf, sz, &nv, 0);
389403
if (err) {
390404
perror(strerror(err));

‎module/zfs/dmu_send.c‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2241,6 +2241,37 @@ setup_send_progress(struct dmu_send_params *dspp)
22412241
return (dssp);
22422242
}
22432243

2244+
/*
2245+
* Payloads must be multiples of 8 bytes for historical compatibility, but
2246+
* XDR-encoded nvlists are sized in multiples of 4 bytes and may need padding.
2247+
*
2248+
* Here we do the simplest possible thing and copy the data to a separate
2249+
* buffer. Not ideal in terms of performance and memory use, but most BEGIN
2250+
* nvlists are small or absent, the allocation is momentary, and we'll need
2251+
* to do this at most once per dataset.
2252+
*
2253+
* It's OK if there is extra data after a packed nvlist on the receiving
2254+
* side because packed nvlists have an internal end-of-list marker.
2255+
*
2256+
* The new buffer is allocated with kmem_alloc() and can be freed with
2257+
* fnvlist_pack_free(), like the original.
2258+
*/
2259+
static inline void
2260+
pad_packed_nvlist(char **buffer, size_t *size)
2261+
{
2262+
size_t size_in = *size;
2263+
size_t extra_bytes = P2ROUNDUP(size_in, 8) - size_in;
2264+
if (extra_bytes != 0) {
2265+
size_t expanded_size = size_in + extra_bytes;
2266+
char *longbuf = kmem_alloc(expanded_size, KM_SLEEP);
2267+
memcpy(longbuf, *buffer, size_in);
2268+
memset(longbuf + size_in, 0, extra_bytes);
2269+
fnvlist_pack_free(*buffer, size_in);
2270+
*buffer = longbuf;
2271+
*size = expanded_size;
2272+
}
2273+
}
2274+
22442275
/*
22452276
* Actually do the bulk of the work in a zfs send.
22462277
*
@@ -2474,7 +2505,7 @@ dmu_send_impl(struct dmu_send_params *dspp)
24742505

24752506
dsl_pool_rele(dp, tag);
24762507

2477-
void *payload = NULL;
2508+
char *payload = NULL;
24782509
size_t payload_len = 0;
24792510
nvlist_t *nvl = fnvlist_alloc();
24802511

@@ -2548,7 +2579,9 @@ dmu_send_impl(struct dmu_send_params *dspp)
25482579
}
25492580

25502581
if (!nvlist_empty(nvl)) {
2551-
payload = fnvlist_pack(nvl, &payload_len);
2582+
VERIFY0(nvlist_pack(nvl, &payload, &payload_len,
2583+
NV_ENCODE_XDR, KM_SLEEP));
2584+
pad_packed_nvlist(&payload, &payload_len);
25522585
drr->drr_payloadlen = payload_len;
25532586
}
25542587

‎tests/runfiles/common.run‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,15 @@ tests = ['scrub_mirror_001_pos', 'scrub_mirror_002_pos',
10251025
'scrub_mirror_003_pos', 'scrub_mirror_004_pos']
10261026
tags = ['functional', 'scrub_mirror']
10271027

1028+
[tests/functional/send_xdr_encoding]
1029+
tests = ['xdr_bookmark_raw', 'xdr_bookmark_raw_with_write',
1030+
'xdr_incr_from_bookmark', 'xdr_incr_from_redacted', 'xdr_raw',
1031+
'xdr_redacted_full', 'xdr_redacted_received',
1032+
'xdr_redacted_received_raw', 'xdr_replication', 'xdr_resume',
1033+
'xdr_resume_bookmark_raw', 'xdr_resume_bookmark_raw_with_write',
1034+
'xdr_resume_raw', 'xdr_resume_redacted']
1035+
tags = ['functional', 'send_xdr_encoding']
1036+
10281037
[tests/functional/slog]
10291038
tests = ['slog_001_pos', 'slog_002_pos', 'slog_003_pos', 'slog_004_pos',
10301039
'slog_005_pos', 'slog_006_pos', 'slog_007_pos', 'slog_008_neg',

‎tests/test-runner/bin/zts-report.py.in‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,8 @@ maybe = {
253253
'renameat2/setup': ['SKIP', renameat2_reason],
254254
'reservation/reservation_008_pos': ['FAIL', 7741],
255255
'reservation/reservation_018_pos': ['FAIL', 5642],
256+
'send_xdr_encoding/xdr_bookmark_raw_with_write': ['FAIL', 18491],
257+
'send_xdr_encoding/xdr_resume_bookmark_raw_with_write': ['FAIL', 18491],
256258
'snapshot/clone_001_pos': ['FAIL', known_reason],
257259
'snapshot/snapshot_006_pos': ['FAIL', known_reason],
258260
'snapshot/snapshot_009_pos': ['FAIL', 7961],

‎tests/zfs-tests/tests/Makefile.am‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,8 @@ nobase_dist_datadir_zfs_tests_tests_DATA += \
376376
functional/rsend/rsend.kshlib \
377377
functional/scrub_mirror/default.cfg \
378378
functional/scrub_mirror/scrub_mirror_common.kshlib \
379+
functional/send_xdr_encoding/send_xdr_encoding.cfg \
380+
functional/send_xdr_encoding/send_xdr_encoding.kshlib \
379381
functional/slog/slog.cfg \
380382
functional/slog/slog.kshlib \
381383
functional/snapshot/snapshot.cfg \
@@ -2129,6 +2131,22 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
21292131
functional/scrub_mirror/scrub_mirror_003_pos.ksh \
21302132
functional/scrub_mirror/scrub_mirror_004_pos.ksh \
21312133
functional/scrub_mirror/setup.ksh \
2134+
functional/send_xdr_encoding/cleanup.ksh \
2135+
functional/send_xdr_encoding/setup.ksh \
2136+
functional/send_xdr_encoding/xdr_bookmark_raw.ksh \
2137+
functional/send_xdr_encoding/xdr_bookmark_raw_with_write.ksh \
2138+
functional/send_xdr_encoding/xdr_incr_from_bookmark.ksh \
2139+
functional/send_xdr_encoding/xdr_incr_from_redacted.ksh \
2140+
functional/send_xdr_encoding/xdr_raw.ksh \
2141+
functional/send_xdr_encoding/xdr_redacted_full.ksh \
2142+
functional/send_xdr_encoding/xdr_redacted_received.ksh \
2143+
functional/send_xdr_encoding/xdr_redacted_received_raw.ksh \
2144+
functional/send_xdr_encoding/xdr_replication.ksh \
2145+
functional/send_xdr_encoding/xdr_resume.ksh \
2146+
functional/send_xdr_encoding/xdr_resume_bookmark_raw.ksh \
2147+
functional/send_xdr_encoding/xdr_resume_bookmark_raw_with_write.ksh \
2148+
functional/send_xdr_encoding/xdr_resume_raw.ksh \
2149+
functional/send_xdr_encoding/xdr_resume_redacted.ksh \
21322150
functional/slog/cleanup.ksh \
21332151
functional/slog/setup.ksh \
21342152
functional/slog/slog_001_pos.ksh \

‎tests/zfs-tests/tests/functional/send_xdr_encoding/cleanup.ksh‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#!/bin/ksh -p
2+
# SPDX-License-Identifier: CDDL-1.0
3+
#
4+
# CDDL HEADER START
5+
#
6+
# This file and its contents are supplied under the terms of the
7+
# Common Development and Distribution License ("CDDL"), version 1.0.
8+
# You may only use this file in accordance with the terms of version
9+
# 1.0 of the CDDL.
10+
#
11+
# A full copy of the text of the CDDL should have accompanied this
12+
# source. A copy of the CDDL is also available via the Internet at
13+
# http://www.illumos.org/license/CDDL.
14+
#
15+
# CDDL HEADER END
16+
#
17+
18+
#
19+
# Copyright (c) 2026 by Garth Snyder. All rights reserved.
20+
#
21+
22+
. $STF_SUITE/tests/functional/send_xdr_encoding/send_xdr_encoding.kshlib
23+
24+
destroy_pool $POOL
25+
destroy_pool $POOL2
26+
27+
log_pass

‎tests/zfs-tests/tests/functional/send_xdr_encoding/send_xdr_encoding.cfg‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# SPDX-License-Identifier: CDDL-1.0
2+
#
3+
# CDDL HEADER START
4+
#
5+
# This file and its contents are supplied under the terms of the
6+
# Common Development and Distribution License ("CDDL"), version 1.0.
7+
# You may only use this file in accordance with the terms of version
8+
# 1.0 of the CDDL.
9+
#
10+
# A full copy of the text of the CDDL should have accompanied this
11+
# source. A copy of the CDDL is also available via the Internet at
12+
# http://www.illumos.org/license/CDDL.
13+
#
14+
# CDDL HEADER END
15+
#
16+
17+
#
18+
# Copyright (c) 2026 by Garth Snyder. All rights reserved.
19+
#
20+
21+
read -r DISK1 DISK2 _ <<<"$DISKS"
22+
export DISK1 DISK2
23+
24+
export POOL=$TESTPOOL
25+
export POOL2=$TESTPOOL2

‎tests/zfs-tests/tests/functional/send_xdr_encoding/send_xdr_encoding.kshlib‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
#!/bin/ksh
2+
# SPDX-License-Identifier: CDDL-1.0
3+
#
4+
# CDDL HEADER START
5+
#
6+
# This file and its contents are supplied under the terms of the
7+
# Common Development and Distribution License ("CDDL"), version 1.0.
8+
# You may only use this file in accordance with the terms of version
9+
# 1.0 of the CDDL.
10+
#
11+
# A full copy of the text of the CDDL should have accompanied this
12+
# source. A copy of the CDDL is also available via the Internet at
13+
# http://www.illumos.org/license/CDDL.
14+
#
15+
# CDDL HEADER END
16+
#
17+
18+
#
19+
# Copyright (c) 2026 by Garth Snyder. All rights reserved.
20+
#
21+
22+
. $STF_SUITE/include/libtest.shlib
23+
. $STF_SUITE/tests/functional/send_xdr_encoding/send_xdr_encoding.cfg
24+
25+
#
26+
# Verify that the DRR_BEGIN records in the given send stream encode their
27+
# nvlist payloads with NV_ENCODE_XDR (and not NV_ENCODE_NATIVE).
28+
#
29+
# DRR_BEGIN records that carry an nvlist payload (raw sends, redacted sends,
30+
# resumed sends, and combinations thereof) must encode that payload with
31+
# NV_ENCODE_XDR so the resulting stream can be portably consumed across
32+
# endianness. Encoding the payload with NV_ENCODE_NATIVE produces a stream
33+
# that is unreadable on a receiver of the opposite endianness.
34+
#
35+
# zstream dump prints a single "nvlist encoding = ..." line per DRR_BEGIN
36+
# record that carries an nvlist payload. The possible values are:
37+
#
38+
# NV_ENCODE_XDR
39+
# NV_ENCODE_NATIVE (big-endian)
40+
# NV_ENCODE_NATIVE (little-endian)
41+
#
42+
# Every test in this suite generates a stream whose DRR_BEGIN record
43+
# carries an nvlist payload, so the pass criterion is:
44+
#
45+
# - At least one NV_ENCODE_XDR line appears, AND
46+
# - No NV_ENCODE_NATIVE line appears.
47+
#
48+
# Requiring at least one XDR line catches the case where zstream dump
49+
# itself fails before producing any encoding output. Asserting on dump
50+
# content rather than dump exit status means a partial dump can still
51+
# fail the test on an NV_ENCODE_NATIVE seen before the failure point.
52+
#
53+
function verify_xdr_nvlist_encoding
54+
{
55+
typeset stream=$1
56+
typeset out
57+
58+
[[ -f "$stream" ]] || \
59+
log_fail "verify_xdr_nvlist_encoding: stream not found: $stream"
60+
61+
out=$(zstream dump "$stream" 2>/dev/null)
62+
63+
if echo "$out" | grep -q 'NV_ENCODE_NATIVE'; then
64+
log_fail "verify_xdr_nvlist_encoding: " \
65+
"NV_ENCODE_NATIVE found in $stream"
66+
fi
67+
if ! echo "$out" | grep -q 'NV_ENCODE_XDR'; then
68+
log_fail "verify_xdr_nvlist_encoding: " \
69+
"no NV_ENCODE_XDR found in $stream"
70+
fi
71+
}

‎tests/zfs-tests/tests/functional/send_xdr_encoding/setup.ksh‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#!/bin/ksh -p
2+
# SPDX-License-Identifier: CDDL-1.0
3+
#
4+
# CDDL HEADER START
5+
#
6+
# This file and its contents are supplied under the terms of the
7+
# Common Development and Distribution License ("CDDL"), version 1.0.
8+
# You may only use this file in accordance with the terms of version
9+
# 1.0 of the CDDL.
10+
#
11+
# A full copy of the text of the CDDL should have accompanied this
12+
# source. A copy of the CDDL is also available via the Internet at
13+
# http://www.illumos.org/license/CDDL.
14+
#
15+
# CDDL HEADER END
16+
#
17+
18+
#
19+
# Copyright (c) 2026 by Garth Snyder. All rights reserved.
20+
#
21+
22+
. $STF_SUITE/tests/functional/send_xdr_encoding/send_xdr_encoding.kshlib
23+
24+
verify_disk_count "$DISKS" 2
25+
26+
create_pool $POOL $DISK1
27+
create_pool $POOL2 $DISK2
28+
29+
log_pass

‎tests/zfs-tests/tests/functional/send_xdr_encoding/xdr_bookmark_raw.ksh‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
#!/bin/ksh -p
2+
# SPDX-License-Identifier: CDDL-1.0
3+
#
4+
# CDDL HEADER START
5+
#
6+
# This file and its contents are supplied under the terms of the
7+
# Common Development and Distribution License ("CDDL"), version 1.0.
8+
# You may only use this file in accordance with the terms of version
9+
# 1.0 of the CDDL.
10+
#
11+
# A full copy of the text of the CDDL should have accompanied this
12+
# source. A copy of the CDDL is also available via the Internet at
13+
# http://www.illumos.org/license/CDDL.
14+
#
15+
# CDDL HEADER END
16+
#
17+
18+
#
19+
# Copyright (c) 2026 by Garth Snyder. All rights reserved.
20+
#
21+
22+
. $STF_SUITE/tests/functional/send_xdr_encoding/send_xdr_encoding.kshlib
23+
24+
#
25+
# Description:
26+
# A raw incremental send from a redaction bookmark on an encrypted dataset
27+
# (zfs send -w -i ds#book ds@snap) carries both BEGINNV_REDACT_FROM_SNAPS
28+
# and crypt_keydata in its DRR_BEGIN nvlist payload. Verify that this
29+
# combined payload is XDR-encoded and the stream can be received.
30+
#
31+
# Strategy:
32+
# 1. Create an encrypted source dataset with a redaction bookmark and a
33+
# later snapshot.
34+
# 2. Establish a raw base on the receiver via zfs send -w of the bookmark's
35+
# source snapshot.
36+
# 3. zfs send -w -i sendfs#book sendfs@s1 to a file.
37+
# 4. Verify that the resulting stream is XDR-encoded.
38+
# 5. Verify that the zfs receive succeeds.
39+
#
40+
41+
verify_runnable "both"
42+
43+
sendfs="$POOL/xdr_bookmark_raw_src"
44+
clonefs="$POOL/xdr_bookmark_raw_clone"
45+
recvfs="$POOL2/xdr_bookmark_raw_recv"
46+
keyfile="/$POOL/xdr_bookmark_raw.key"
47+
full_stream="/$POOL/xdr_bookmark_raw_full.zsend"
48+
incr_stream="/$POOL/xdr_bookmark_raw_incr.zsend"
49+
50+
function cleanup
51+
{
52+
datasetexists $sendfs && destroy_dataset $sendfs -R
53+
datasetexists $recvfs && destroy_dataset $recvfs -R
54+
rm -f $keyfile $full_stream $incr_stream
55+
}
56+
log_onexit cleanup
57+
58+
log_assert "BEGIN nvlist of a raw incremental from a redaction bookmark is " \
59+
"XDR-encoded and receivable"
60+
61+
log_must eval "echo 'thisisapassphrase' > $keyfile"
62+
log_must zfs create -o encryption=on -o keyformat=passphrase \
63+
-o keylocation=file://$keyfile $sendfs
64+
65+
log_must dd if=/dev/urandom of=/$sendfs/f1 bs=128k count=8 status=none
66+
log_must dd if=/dev/urandom of=/$sendfs/f2 bs=128k count=8 status=none
67+
log_must zfs snapshot $sendfs@s0
68+
69+
# The clone inherits encryption from $sendfs.
70+
log_must zfs clone $sendfs@s0 $clonefs
71+
log_must dd if=/dev/urandom of=/$clonefs/f1 bs=128k count=8 conv=notrunc \
72+
status=none
73+
log_must zfs snapshot $clonefs@s
74+
75+
log_must zfs redact $sendfs@s0 redaction-bookmark $clonefs@s
76+
77+
# Take @s1 with no intervening writes. See xdr_bookmark_raw_with_write.ksh
78+
# for a variant that includes a post-redact write; that variant exercises
79+
# a known kernel-side issue (#18491) and may flake.
80+
log_must zfs snapshot $sendfs@s1
81+
82+
# Establish a raw base on the receiver.
83+
log_must eval "zfs send -w $sendfs@s0 > $full_stream"
84+
log_must eval "zfs receive $recvfs < $full_stream"
85+
86+
# Raw incremental from the redaction bookmark. This is the test focus.
87+
log_must eval "zfs send -w -i $sendfs#redaction-bookmark $sendfs@s1 > \
88+
$incr_stream"
89+
verify_xdr_nvlist_encoding $incr_stream
90+
log_must eval "zfs receive $recvfs < $incr_stream"
91+
92+
log_pass "BEGIN nvlist of a raw incremental from a redaction bookmark is " \
93+
"XDR-encoded and receivable"

0 commit comments

Comments
 (0)