* [PATCH v2 0/3]: generic/020: fix MAX_ATTRVAL_SIZE values
@ 2022-04-12 10:49 David Disseldorp
2022-04-12 10:49 ` [PATCH v2 1/3] common/attr: add and use _attr_get_max() David Disseldorp
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Disseldorp @ 2022-04-12 10:49 UTC (permalink / raw)
To: fstests; +Cc: dchinner
MAX_ATTRVAL_SIZE provides a per-filesystem maximum xattr value length
limit. There are a few problems with the current values:
- XFS, UDF and Btrfs are incorrectly hardcoded to use 64 bytes
+ This is a regression from the larger value used prior to fff4359d
+ Btrfs's should be calculated using nodesize and xattr name length
- NFS currently uses a 64K limit
+ This may be above the server's underlying failsystem limit, so use
a conservative estimate instead
Cheers, David
common/attr | 175 ++++++++++++++++++++++++------------------
tests/generic/020 | 10 ++-
tests/generic/020.out | 2 +-
3 files changed, 108 insertions(+), 79 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] common/attr: add and use _attr_get_max()
2022-04-12 10:49 [PATCH v2 0/3]: generic/020: fix MAX_ATTRVAL_SIZE values David Disseldorp
@ 2022-04-12 10:49 ` David Disseldorp
2022-04-13 0:31 ` Dave Chinner
2022-04-12 10:50 ` [PATCH v2 2/3] common/attr: fix MAX_ATTRVAL_SIZE for XFS, UDF, Btrfs and NFS David Disseldorp
2022-04-12 10:50 ` [PATCH v2 3/3] generic/020: fix unaligned MAX_ATTRVAL_SIZE output filter David Disseldorp
2 siblings, 1 reply; 6+ messages in thread
From: David Disseldorp @ 2022-04-12 10:49 UTC (permalink / raw)
To: fstests; +Cc: dchinner, David Disseldorp
No functional change. The MAX_ATTRS and MAX_ATTRVAL_SIZE exports are
only used by generic/020. In preparation for taking into account the
attr name length when calculating MAX_ATTRVAL_SIZE, split the current
logic out into a _attr_get_max() helper function.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
common/attr | 163 +++++++++++++++++++++++++---------------------
tests/generic/020 | 10 ++-
2 files changed, 95 insertions(+), 78 deletions(-)
diff --git a/common/attr b/common/attr
index dae8a1bb..a80b10a1 100644
--- a/common/attr
+++ b/common/attr
@@ -264,80 +264,93 @@ _getfattr()
return ${PIPESTATUS[0]}
}
-# set maximum total attr space based on fs type
-case "$FSTYP" in
-xfs|udf|pvfs2|9p|ceph|nfs)
- MAX_ATTRS=1000
- ;;
-ext2|ext3|ext4)
- # For 4k blocksizes, most of the attributes have an attr_name of
- # "attribute_NN" which is 12, and "value_NN" which is 8.
- # But for larger block sizes, we start having extended attributes of the
- # form "attribute_NNN" or "attribute_NNNN", and "value_NNN" and
- # "value_NNNN", which causes the round(len(..), 4) to jump up by 4
- # bytes. So round_up(len(attr_name, 4)) becomes 16 instead of 12, and
- # round_up(len(value, 4)) becomes 12 instead of 8.
- #
- # For 64K blocksize the calculation becomes
- # max_attrs = (block_size - 32) / (16 + 12 + 16)
- # or
- # max_attrs = (block_size - 32) / 44
- #
- # For 4K blocksize:-
- # max_attrs = (block_size - 32) / (16 + 8 + 12)
- # or
- # max_attrs = (block_size - 32) / 36
- #
- # Note (for 4K bs) above are exact calculations for attrs of type
- # attribute_NN with values of type value_NN.
- # With above calculations, for 4k blocksize max_attrs becomes 112.
- # This means we can have few attrs of type attribute_NNN with values of
- # type value_NNN. To avoid/handle this we need to add extra 4 bytes of
- # headroom.
- #
- # So for 4K, the calculations becomes:-
- # max_attrs = (block_size - 32) / (16 + 8 + 12 + 4)
- # or
- # max_attrs = (block_size - 32) / 40
- #
- # Assume max ~1 block of attrs
- BLOCK_SIZE=`_get_block_size $TEST_DIR`
- if [ $BLOCK_SIZE -le 4096 ]; then
- let MAX_ATTRS=$((($BLOCK_SIZE - 32) / (16 + 8 + 12 + 4)))
- else
- let MAX_ATTRS=$((($BLOCK_SIZE - 32) / (16 + 12 + 16 )))
- fi
- ;;
-*)
- # Assume max ~1 block of attrs
- BLOCK_SIZE=`_get_block_size $TEST_DIR`
- # user.attribute_XXX="value.XXX" is about 32 bytes; leave some overhead
- let MAX_ATTRS=$BLOCK_SIZE/40
-esac
-
-export MAX_ATTRS
-
-# Set max attr value size based on fs type
-case "$FSTYP" in
-xfs|udf|btrfs)
- MAX_ATTRVAL_SIZE=64
- ;;
-pvfs2)
- MAX_ATTRVAL_SIZE=8192
- ;;
-9p|ceph|nfs)
- MAX_ATTRVAL_SIZE=65536
- ;;
-bcachefs)
- MAX_ATTRVAL_SIZE=1024
- ;;
-*)
- # Assume max ~1 block of attrs
- BLOCK_SIZE=`_get_block_size $TEST_DIR`
- # leave a little overhead
- let MAX_ATTRVAL_SIZE=$BLOCK_SIZE-256
-esac
-
-export MAX_ATTRVAL_SIZE
+# export fs-specific MAX_ATTRS and MAX_ATTRVAL_SIZE values. The parameter
+# @max_attrval_namelen is required for filesystems which take into account attr
+# name lengths (including namespace prefix) when determining limits.
+_attr_get_max()
+{
+ local max_attrval_namelen="$1"
+
+ # set maximum total attr space based on fs type
+ case "$FSTYP" in
+ xfs|udf|pvfs2|9p|ceph|nfs)
+ MAX_ATTRS=1000
+ ;;
+ ext2|ext3|ext4)
+ # For 4k blocksizes, most of the attributes have an attr_name of
+ # "attribute_NN" which is 12, and "value_NN" which is 8.
+ # But for larger block sizes, we start having extended
+ # attributes of the
+ # form "attribute_NNN" or "attribute_NNNN", and "value_NNN" and
+ # "value_NNNN", which causes the round(len(..), 4) to jump up by
+ # 4 bytes. So round_up(len(attr_name, 4)) becomes 16 instead of
+ # 12, and round_up(len(value, 4)) becomes 12 instead of 8.
+ #
+ # For 64K blocksize the calculation becomes
+ # max_attrs = (block_size - 32) / (16 + 12 + 16)
+ # or
+ # max_attrs = (block_size - 32) / 44
+ #
+ # For 4K blocksize:-
+ # max_attrs = (block_size - 32) / (16 + 8 + 12)
+ # or
+ # max_attrs = (block_size - 32) / 36
+ #
+ # Note (for 4K bs) above are exact calculations for attrs of
+ # type attribute_NN with values of type value_NN.
+ # With above calculations, for 4k blocksize max_attrs becomes
+ # 112.
+ # This means we can have few attrs of type attribute_NNN with
+ # values of
+ # type value_NNN. To avoid/handle this we need to add extra 4
+ # bytes of headroom.
+ #
+ # So for 4K, the calculations becomes:-
+ # max_attrs = (block_size - 32) / (16 + 8 + 12 + 4)
+ # or
+ # max_attrs = (block_size - 32) / 40
+ #
+ # Assume max ~1 block of attrs
+ BLOCK_SIZE=`_get_block_size $TEST_DIR`
+ if [ $BLOCK_SIZE -le 4096 ]; then
+ let MAX_ATTRS=$((($BLOCK_SIZE - 32) / (16 + 8 + 12 + 4)))
+ else
+ let MAX_ATTRS=$((($BLOCK_SIZE - 32) / (16 + 12 + 16 )))
+ fi
+ ;;
+ *)
+ # Assume max ~1 block of attrs
+ BLOCK_SIZE=`_get_block_size $TEST_DIR`
+ # user.attribute_XXX="value.XXX" is about 32 bytes; leave some
+ # overhead
+ let MAX_ATTRS=$BLOCK_SIZE/40
+ esac
+
+ export MAX_ATTRS
+
+ # Set max attr value size based on fs type
+ case "$FSTYP" in
+ xfs|udf|btrfs)
+ MAX_ATTRVAL_SIZE=64
+ ;;
+ pvfs2)
+ MAX_ATTRVAL_SIZE=8192
+ ;;
+ 9p|ceph|nfs)
+ MAX_ATTRVAL_SIZE=65536
+ ;;
+ bcachefs)
+ MAX_ATTRVAL_SIZE=1024
+ ;;
+ *)
+ # Assume max ~1 block of attrs
+ BLOCK_SIZE=`_get_block_size $TEST_DIR`
+ # leave a little overhead
+ let MAX_ATTRVAL_SIZE=$BLOCK_SIZE-256
+ esac
+
+ export MAX_ATTRVAL_SIZE
+}
+
# make sure this script returns success
/bin/true
diff --git a/tests/generic/020 b/tests/generic/020
index 29ef853c..34861401 100755
--- a/tests/generic/020
+++ b/tests/generic/020
@@ -88,6 +88,9 @@ _attr_list $testfile
echo "*** add lots of attributes"
v=0
+max_attrval_name="long_attr" # add 5 for "user." prefix
+_attr_get_max "$(( 5 + ${#max_attrval_name} ))"
+
while [ $v -lt $MAX_ATTRS ]
do
echo -n "value_$v" | attr -s "attribute_$v" $testfile >>$seqres.full
@@ -128,11 +131,12 @@ _attr_list $testfile
echo "*** really long value"
dd if=/dev/zero bs=1 count=$MAX_ATTRVAL_SIZE 2>/dev/null \
- | _attr -s "long_attr" $testfile >/dev/null
+ | _attr -s "$max_attrval_name" $testfile >/dev/null
OCTAL_SIZE=`echo "obase=8; $MAX_ATTRVAL_SIZE" | bc`
-_attr -q -g "long_attr" $testfile | od -t x1 | sed -e "s/^0*$OCTAL_SIZE$/ATTRSIZE/"
-_attr -r "long_attr" $testfile >/dev/null
+_attr -q -g "$max_attrval_name" $testfile | od -t x1 \
+ | sed -e "s/^0*$OCTAL_SIZE$/ATTRSIZE/"
+_attr -r "$max_attrval_name" $testfile >/dev/null
echo "*** set/get/remove really long names (expect failure)"
short="XXXXXXXXXX"
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] common/attr: fix MAX_ATTRVAL_SIZE for XFS, UDF, Btrfs and NFS
2022-04-12 10:49 [PATCH v2 0/3]: generic/020: fix MAX_ATTRVAL_SIZE values David Disseldorp
2022-04-12 10:49 ` [PATCH v2 1/3] common/attr: add and use _attr_get_max() David Disseldorp
@ 2022-04-12 10:50 ` David Disseldorp
2022-04-12 10:50 ` [PATCH v2 3/3] generic/020: fix unaligned MAX_ATTRVAL_SIZE output filter David Disseldorp
2 siblings, 0 replies; 6+ messages in thread
From: David Disseldorp @ 2022-04-12 10:50 UTC (permalink / raw)
To: fstests; +Cc: dchinner, David Disseldorp
As found by Dave Chinner, fff4359d ("020: make this xattr test generic")
unintentionally changed the long attribute value length from 100K to 64
*bytes* for XFS, UDF and Btrfs.
Update XFS and UDF to use 64K MAX_ATTRVAL_SIZE value. For Btrfs, use the
nodesize, xattr length and tree entry overhead sizes to calculate the
maximum.
NFS doesn't provide a way to find out the MAX_ATTRVAL_SIZE for the
underlying filesystem on the server, so just use a rough 1K limit.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
common/attr | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/common/attr b/common/attr
index a80b10a1..1e1c8f6e 100644
--- a/common/attr
+++ b/common/attr
@@ -328,20 +328,32 @@ _attr_get_max()
export MAX_ATTRS
- # Set max attr value size based on fs type
+ # Set max attr value size in bytes based on fs type
case "$FSTYP" in
- xfs|udf|btrfs)
- MAX_ATTRVAL_SIZE=64
+ btrfs)
+ _require_btrfs_command inspect-internal dump-super
+ local ns=$($BTRFS_UTIL_PROG inspect-internal dump-super \
+ $TEST_DEV | sed -n 's/nodesize\s*\(.*\)/\1/p')
+ [ -n "$ns" ] || _fail "failed to obtain nodesize"
+ # max == nodesize - sizeof(struct btrfs_header)
+ # - sizeof(struct btrfs_item)
+ # - sizeof(struct btrfs_dir_item) - name_len
+ MAX_ATTRVAL_SIZE=$(( $ns - 156 - $max_attrval_namelen ))
;;
pvfs2)
MAX_ATTRVAL_SIZE=8192
;;
- 9p|ceph|nfs)
+ xfs|udf|9p|ceph)
MAX_ATTRVAL_SIZE=65536
;;
bcachefs)
MAX_ATTRVAL_SIZE=1024
;;
+ nfs)
+ # NFS doesn't provide a way to find out the MAX_ATTRVAL_SIZE for
+ # the underlying filesystem, so just use the lowest value above.
+ MAX_ATTRVAL_SIZE=1024
+ ;;
*)
# Assume max ~1 block of attrs
BLOCK_SIZE=`_get_block_size $TEST_DIR`
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] generic/020: fix unaligned MAX_ATTRVAL_SIZE output filter
2022-04-12 10:49 [PATCH v2 0/3]: generic/020: fix MAX_ATTRVAL_SIZE values David Disseldorp
2022-04-12 10:49 ` [PATCH v2 1/3] common/attr: add and use _attr_get_max() David Disseldorp
2022-04-12 10:50 ` [PATCH v2 2/3] common/attr: fix MAX_ATTRVAL_SIZE for XFS, UDF, Btrfs and NFS David Disseldorp
@ 2022-04-12 10:50 ` David Disseldorp
2022-04-13 0:32 ` Dave Chinner
2 siblings, 1 reply; 6+ messages in thread
From: David Disseldorp @ 2022-04-12 10:50 UTC (permalink / raw)
To: fstests; +Cc: dchinner, David Disseldorp
The current attr -g "$max_attrval_name" output filter is broken if
MAX_ATTRVAL_SIZE isn't 16-byte aligned, due to od's duplicate
suppression behaviour.
Fix it by having od only dump one byte per line.
Signed-off-by: David Disseldorp <ddiss@suse.de>
---
tests/generic/020 | 2 +-
tests/generic/020.out | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/generic/020 b/tests/generic/020
index 34861401..d1913953 100755
--- a/tests/generic/020
+++ b/tests/generic/020
@@ -134,7 +134,7 @@ dd if=/dev/zero bs=1 count=$MAX_ATTRVAL_SIZE 2>/dev/null \
| _attr -s "$max_attrval_name" $testfile >/dev/null
OCTAL_SIZE=`echo "obase=8; $MAX_ATTRVAL_SIZE" | bc`
-_attr -q -g "$max_attrval_name" $testfile | od -t x1 \
+_attr -q -g "$max_attrval_name" $testfile | od -w1 -t x1 \
| sed -e "s/^0*$OCTAL_SIZE$/ATTRSIZE/"
_attr -r "$max_attrval_name" $testfile >/dev/null
diff --git a/tests/generic/020.out b/tests/generic/020.out
index 7e3e65bd..0dc5e09f 100644
--- a/tests/generic/020.out
+++ b/tests/generic/020.out
@@ -47,7 +47,7 @@ user.snrub="fish2\012"
user.snrub="fish2\012"
*** really long value
-0000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+0000000 00
*
ATTRSIZE
*** set/get/remove really long names (expect failure)
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] common/attr: add and use _attr_get_max()
2022-04-12 10:49 ` [PATCH v2 1/3] common/attr: add and use _attr_get_max() David Disseldorp
@ 2022-04-13 0:31 ` Dave Chinner
0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2022-04-13 0:31 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, dchinner
On Tue, Apr 12, 2022 at 12:49:59PM +0200, David Disseldorp wrote:
> No functional change. The MAX_ATTRS and MAX_ATTRVAL_SIZE exports are
> only used by generic/020. In preparation for taking into account the
> attr name length when calculating MAX_ATTRVAL_SIZE, split the current
> logic out into a _attr_get_max() helper function.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> common/attr | 163 +++++++++++++++++++++++++---------------------
> tests/generic/020 | 10 ++-
> 2 files changed, 95 insertions(+), 78 deletions(-)
Hmmm. when you said you were going to make these helper functions,
I thought you meant you were going to move them to generic/020
as helper functions as that is the only place that uses them.
I don't see any point in making them single use helper functions
specific to a single test and then leaving them in common/attr....
Then you can get rid of the exported variable and use a test local
variable for MAX_ATTRS and MAX_ATTRVAL_SIZE....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] generic/020: fix unaligned MAX_ATTRVAL_SIZE output filter
2022-04-12 10:50 ` [PATCH v2 3/3] generic/020: fix unaligned MAX_ATTRVAL_SIZE output filter David Disseldorp
@ 2022-04-13 0:32 ` Dave Chinner
0 siblings, 0 replies; 6+ messages in thread
From: Dave Chinner @ 2022-04-13 0:32 UTC (permalink / raw)
To: David Disseldorp; +Cc: fstests, dchinner
On Tue, Apr 12, 2022 at 12:50:01PM +0200, David Disseldorp wrote:
> The current attr -g "$max_attrval_name" output filter is broken if
> MAX_ATTRVAL_SIZE isn't 16-byte aligned, due to od's duplicate
> suppression behaviour.
> Fix it by having od only dump one byte per line.
>
> Signed-off-by: David Disseldorp <ddiss@suse.de>
> ---
> tests/generic/020 | 2 +-
> tests/generic/020.out | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
looks good.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-13 0:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-12 10:49 [PATCH v2 0/3]: generic/020: fix MAX_ATTRVAL_SIZE values David Disseldorp
2022-04-12 10:49 ` [PATCH v2 1/3] common/attr: add and use _attr_get_max() David Disseldorp
2022-04-13 0:31 ` Dave Chinner
2022-04-12 10:50 ` [PATCH v2 2/3] common/attr: fix MAX_ATTRVAL_SIZE for XFS, UDF, Btrfs and NFS David Disseldorp
2022-04-12 10:50 ` [PATCH v2 3/3] generic/020: fix unaligned MAX_ATTRVAL_SIZE output filter David Disseldorp
2022-04-13 0:32 ` Dave Chinner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.