[PATCH blktests 0/2] zbd: Support dm-crypt

[PATCH blktests 0/2] zbd: Support dm-crypt

[Shin'ichiro Kawasaki]

Linux kernel 5.9 added zoned block device support to dm-crypt. With this zoned
dm-crypt device, zbd group test cases pass except zbd/007. This series makes
required changes to allow the test case zbd/007 pass with zoned dm-cyrpt
devices. The first patch adds dm-crypt support to the helper function
_get_dev_container_and_sector(). The second patch wipes out broken data on the
dm-crypt devices which was left after the test case run.

Shin'ichiro Kawasaki (2):
  zbd/rc: Support dm-crypt
  zbd/007: Reset test target zones at test end

 tests/zbd/007 |  7 +++++++
 tests/zbd/rc  | 20 ++++++++++++++------
 2 files changed, 21 insertions(+), 6 deletions(-)

-- 
2.31.1

[PATCH blktests 1/2] zbd/rc: Support dm-crypt

[Shin'ichiro Kawasaki]

Linux kernel 5.9 added zoned block device support to dm-crypt. To test
dm-crypt devices, modify the function _get_dev_container_and_sector().
To handle device-mapper table format difference between dm-crypt and
dm-linear/flakey, add dev_idx and off_idx local variables.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/zbd/rc | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/zbd/rc b/tests/zbd/rc
index 1237363..9deadc1 100644
--- a/tests/zbd/rc
+++ b/tests/zbd/rc
@@ -327,6 +327,7 @@ _get_dev_container_and_sector() {
 	local cont_dev
 	local -i offset
 	local -a tbl_line
+	local -i dev_idx=3 off_idx=4
 
 	if _test_dev_is_partition; then
 		offset=$(<"${TEST_DEV_PART_SYSFS}/start")
@@ -340,13 +341,19 @@ _get_dev_container_and_sector() {
 		return 1
 	fi
 	if ! _test_dev_has_dm_map linear &&
-			! _test_dev_has_dm_map flakey; then
-		echo -n "dm mapping test other than linear/flakey is"
+			! _test_dev_has_dm_map flakey &&
+			! _test_dev_has_dm_map crypt; then
+		echo -n "dm mapping test other than linear/flakey/crypt is"
 		echo "not implemented"
 		return 1
 	fi
 
-	# Parse dm table lines for dm-linear or dm-flakey target
+	if _test_dev_has_dm_map crypt; then
+		dev_idx=6
+		off_idx=7
+	fi
+
+	# Parse dm table lines for dm-linear, dm-flakey or dm-crypt target
 	while read -r -a tbl_line; do
 		local -i map_start=${tbl_line[0]}
 		local -i map_end=$((tbl_line[0] + tbl_line[1]))
@@ -355,10 +362,11 @@ _get_dev_container_and_sector() {
 			continue
 		fi
 
-		offset=${tbl_line[4]}
-		if ! cont_dev=$(_get_dev_path_by_id "${tbl_line[3]}"); then
+		offset=${tbl_line[off_idx]}
+		if ! cont_dev=$(_get_dev_path_by_id \
+					"${tbl_line[dev_idx]}"); then
 			echo -n "Cannot access to container device: "
-			echo "${tbl_line[3]}"
+			echo "${tbl_line[dev_idx]}"
 			return 1
 		fi
 
-- 
2.31.1

Re: [PATCH blktests 1/2] zbd/rc: Support dm-crypt

[Bart Van Assche]

On 7/13/21 3:12 AM, Shin'ichiro Kawasaki wrote:
> -	# Parse dm table lines for dm-linear or dm-flakey target
> +	if _test_dev_has_dm_map crypt; then
> +		dev_idx=6
> +		off_idx=7
> +	fi
> +
> +	# Parse dm table lines for dm-linear, dm-flakey or dm-crypt target
>  	while read -r -a tbl_line; do
>  		local -i map_start=${tbl_line[0]}
>  		local -i map_end=$((tbl_line[0] + tbl_line[1]))
> @@ -355,10 +362,11 @@ _get_dev_container_and_sector() {
>  			continue
>  		fi
>  
> -		offset=${tbl_line[4]}
> -		if ! cont_dev=$(_get_dev_path_by_id "${tbl_line[3]}"); then
> +		offset=${tbl_line[off_idx]}
> +		if ! cont_dev=$(_get_dev_path_by_id \
> +					"${tbl_line[dev_idx]}"); then
>  			echo -n "Cannot access to container device: "
> -			echo "${tbl_line[3]}"
> +			echo "${tbl_line[dev_idx]}"
>  			return 1
>  		fi

To me the above code looks like code that is hard to maintain. Can the
above parser be replaced by reading /sys/block/*/slaves? An example from
my workstation for dm-crypt:

$ ls /sys/block/dm-0/slaves
nvme0n1p2

Thanks,

Bart.

Re: [PATCH blktests 1/2] zbd/rc: Support dm-crypt

[Shinichiro Kawasaki]

On Jul 13, 2021 / 06:59, Bart Van Assche wrote:
> On 7/13/21 3:12 AM, Shin'ichiro Kawasaki wrote:
> > -	# Parse dm table lines for dm-linear or dm-flakey target
> > +	if _test_dev_has_dm_map crypt; then
> > +		dev_idx=6
> > +		off_idx=7
> > +	fi
> > +
> > +	# Parse dm table lines for dm-linear, dm-flakey or dm-crypt target
> >  	while read -r -a tbl_line; do
> >  		local -i map_start=${tbl_line[0]}
> >  		local -i map_end=$((tbl_line[0] + tbl_line[1]))
> > @@ -355,10 +362,11 @@ _get_dev_container_and_sector() {
> >  			continue
> >  		fi
> >  
> > -		offset=${tbl_line[4]}
> > -		if ! cont_dev=$(_get_dev_path_by_id "${tbl_line[3]}"); then
> > +		offset=${tbl_line[off_idx]}
> > +		if ! cont_dev=$(_get_dev_path_by_id \
> > +					"${tbl_line[dev_idx]}"); then
> >  			echo -n "Cannot access to container device: "
> > -			echo "${tbl_line[3]}"
> > +			echo "${tbl_line[dev_idx]}"
> >  			return 1
> >  		fi
> 
> To me the above code looks like code that is hard to maintain. Can the
> above parser be replaced by reading /sys/block/*/slaves? An example from
> my workstation for dm-crypt:
> 
> $ ls /sys/block/dm-0/slaves
> nvme0n1p2

Hi, Bart. Thanks for the suggestion, but I don't think sysfs slaves attribute
will simplify the code. The helper function _get_dev_container_and_sector()
requires pairs of 'map starting sector offset' and 'map target device'. Dm
table provides both of them, but the sysfs slaves attribute provides only the
map target device. As far as I understand, the helper function must parse the
dm table anyway to get the map starting sector offsets, and paired devices.

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH blktests 2/2] zbd/007: Reset test target zones at test end

[Shin'ichiro Kawasaki]

The test case zbd/007 checks write pointer mapping between a logical
device and its container device. To do so, it moves write pointers of
the container device by writing data to the container device. When the
logical device is a dm-crypt device, this test case works as expected,
but the data written to the container device is not encrypted, then it
leaves broken data on the logical, dm-crypt device. This results in I/O
errors in the following operations to the dm-crypt device.

To avoid the I/O errors, reset the test target zones of the logical
device at the test case end to wipe out the broken data.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/zbd/007 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/zbd/007 b/tests/zbd/007
index 9d7ca67..2179647 100755
--- a/tests/zbd/007
+++ b/tests/zbd/007
@@ -109,5 +109,12 @@ test_device() {
 	done
 	_put_blkzone_report
 
+	# When the logical devices is dm-crypt, the write pointer moves on
+	# its container device break data contents on the logical device. Reset
+	# zones of the logical device to wipe out the broken data.
+	for ((i=0; i < ${#test_z[@]}; i++)); do
+		blkzone reset -o "${test_z_start[i]}" -c 1 "${TEST_DEV}"
+	done
+
 	echo "Test complete"
 }
-- 
2.31.1

Re: [PATCH blktests 0/2] zbd: Support dm-crypt

[Omar Sandoval]

On Tue, Jul 13, 2021 at 07:12:37PM +0900, Shin'ichiro Kawasaki wrote:
> Linux kernel 5.9 added zoned block device support to dm-crypt. With this zoned
> dm-crypt device, zbd group test cases pass except zbd/007. This series makes
> required changes to allow the test case zbd/007 pass with zoned dm-cyrpt
> devices. The first patch adds dm-crypt support to the helper function
> _get_dev_container_and_sector(). The second patch wipes out broken data on the
> dm-crypt devices which was left after the test case run.
> 
> Shin'ichiro Kawasaki (2):
>   zbd/rc: Support dm-crypt
>   zbd/007: Reset test target zones at test end
> 
>  tests/zbd/007 |  7 +++++++
>  tests/zbd/rc  | 20 ++++++++++++++------
>  2 files changed, 21 insertions(+), 6 deletions(-)

The patches look reasonable. Could you provide instructions on how to
run with zoned dm-crypt so I can run the tests?

Re: [PATCH blktests 0/2] zbd: Support dm-crypt

[Shinichiro Kawasaki]

On Jul 26, 2021 / 12:02, Omar Sandoval wrote:
> On Tue, Jul 13, 2021 at 07:12:37PM +0900, Shin'ichiro Kawasaki wrote:
> > Linux kernel 5.9 added zoned block device support to dm-crypt. With this zoned
> > dm-crypt device, zbd group test cases pass except zbd/007. This series makes
> > required changes to allow the test case zbd/007 pass with zoned dm-cyrpt
> > devices. The first patch adds dm-crypt support to the helper function
> > _get_dev_container_and_sector(). The second patch wipes out broken data on the
> > dm-crypt devices which was left after the test case run.
> > 
> > Shin'ichiro Kawasaki (2):
> >   zbd/rc: Support dm-crypt
> >   zbd/007: Reset test target zones at test end
> > 
> >  tests/zbd/007 |  7 +++++++
> >  tests/zbd/rc  | 20 ++++++++++++++------
> >  2 files changed, 21 insertions(+), 6 deletions(-)
> 
> The patches look reasonable. Could you provide instructions on how to
> run with zoned dm-crypt so I can run the tests?

Sure, the script below sets up a null_blk zoned and dm-crypt on top of it.

--- start ---
# create zoned null_blk device as /dev/nullb0
rmmod null_blk
modprobe null_blk nr_devices=0
cd /sys/kernel/config/nullb
mkdir nullb0
cd nullb0
echo 1 > zoned
echo 1 > memory_backed
echo 1 > power

# set up dm-crypt on /dev/nullb0
KEY_FILE=/tmp/dmckey
dd if=/dev/random of="${KEY_FILE}" bs=1 count=512 > /dev/null 2>&1
cryptsetup open --batch-mode --type plain --key-file "${KEY_FILE}" \
	   /dev/nullb0 dmctest
--- end ---

Maybe no need to mention but I used blktests config as follows. With these set
up, I confirmed that block group and zbd group test cases run without failures.

--- start ---
TEST_DEVS=(/dev/mapper/dmctest)
DEVICE_ONLY=1
--- end ---

FYI, two commands below clean up the device.

# dmsetup remove dmctest
# rmdir /sys/kernel/config/nullb/nullb0

-- 
Best Regards,
Shin'ichiro Kawasaki