On 5/7/25 11:48 PM, Verma, Vishal L wrote:
> On Wed, 2025-05-07 at 09:46 -0700, Dave Jiang wrote:
>> Current host bridge validation in cxl-topology.sh assumes that the
>> decoder enumeration is in order and therefore the port numbers can
>> be used as a sorting key. With delayed port enumeration, this
>> assumption is no longer true. Change the sorting to by number
>> of children ports for each host bridge as the test code expects
>> the first 2 host bridges to have 2 children and the third to only
>> have 1.
>>
>> Signed-off-by: Dave Jiang <dave.ji...@intel.com>
>> ---
>> test/cxl-topology.sh | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
>> index 90b9c98273db..41d6f052394d 100644
>> --- a/test/cxl-topology.sh
>> +++ b/test/cxl-topology.sh
>> @@ -37,15 +37,16 @@ root=$(jq -r ".[] | .bus" <<< $json)
>>
>>
>> # validate 2 or 3 host bridges under a root port
>> -port_sort="sort_by(.port | .[4:] | tonumber)"
>> json=$($CXL list -b cxl_test -BP)
>> count=$(jq ".[] | .[\"ports:$root\"] | length" <<< $json)
>> ((count == 2)) || ((count == 3)) || err "$LINENO"
>> bridges=$count
>>
>> -bridge[0]=$(jq -r ".[] | .[\"ports:$root\"] | $port_sort | .[0].port" <<<
>> $json)
>> -bridge[1]=$(jq -r ".[] | .[\"ports:$root\"] | $port_sort | .[1].port" <<<
>> $json)
>> -((bridges > 2)) && bridge[2]=$(jq -r ".[] | .[\"ports:$root\"] | $port_sort
>> | .[2].port" <<< $json)
>> +bridge[0]=$(jq -r --arg key "$root" '.[] | select(has("ports:" + $key)) |
>> .["ports:" + $key] | map({full: ., length: (.["ports:" + .port] | length)})
>> | sort_by(-.length) | map(.full) | .[0].port' <<< "$json")
>> +
>> +bridge[1]=$(jq -r --arg key "$root" '.[] | select(has("ports:" + $key)) |
>> .["ports:" + $key] | map({full: ., length: (.["ports:" + .port] | length)})
>> | sort_by(-.length) | map(.full) | .[1].port' <<< "$json")
>> +
>> +((bridges > 2)) && bridge[2]=$(jq -r --arg key "$root" '.[] |
>> select(has("ports:" + $key)) | .["ports:" + $key] | map({full: ., length:
>> (.["ports:" + .port] | length)}) | sort_by(-.length) | map(.full) |
>> .[2].port' <<< "$json")
>>
>
> The jq filtering looks reasonable, but the long lines are definitely a
> bit unsightly, not to mention hard to follow/edit in the future.
>
> How about this incremental patch (It passes the test for me with the
> delayed port enumeration series):
LGTM
DJ
>
> -->8--
>
> From 996610fac0a751acc5d68a56ff7d6746348a80c4 Mon Sep 17 00:00:00 2001
> From: Vishal Verma <vishal.l.ve...@intel.com>
> Date: Thu, 8 May 2025 00:46:27 -0600
> Subject: [ndctl PATCH] test: Cleanup long lines in cxl-topology.sh
>
> Signed-off-by: Vishal Verma <vishal.l.ve...@intel.com>
> ---
> test/cxl-topology.sh | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
> index f4ba7374..b68cb8b2 100644
> --- a/test/cxl-topology.sh
> +++ b/test/cxl-topology.sh
> @@ -42,11 +42,32 @@ count=$(jq ".[] | .[\"ports:$root\"] | length" <<< $json)
> ((count == 2)) || ((count == 3)) || err "$LINENO"
> bridges=$count
>
> -bridge[0]=$(jq -r --arg key "$root" '.[] | select(has("ports:" + $key)) |
> .["ports:" + $key] | map({full: ., length: (.["ports:" + .port] | length)}) |
> sort_by(-.length) | map(.full) | .[0].port' <<< "$json")
> +bridge_filter()
> +{
> + local br_num="$1"
>
> -bridge[1]=$(jq -r --arg key "$root" '.[] | select(has("ports:" + $key)) |
> .["ports:" + $key] | map({full: ., length: (.["ports:" + .port] | length)}) |
> sort_by(-.length) | map(.full) | .[1].port' <<< "$json")
> + jq -r \
> + --arg key "$root" \
> + --argjson br_num "$br_num" \
> + '.[] |
> + select(has("ports:" + $key)) |
> + .["ports:" + $key] |
> + map(
> + {
> + full: .,
> + length: (.["ports:" + .port] | length)
> + }
> + ) |
> + sort_by(-.length) |
> + map(.full) |
> + .[$br_num].port'
> +}
>
> -((bridges > 2)) && bridge[2]=$(jq -r --arg key "$root" '.[] |
> select(has("ports:" + $key)) | .["ports:" + $key] | map({full: ., length:
> (.["ports:" + .port] | length)}) | sort_by(-.length) | map(.full) |
> .[2].port' <<< "$json")
> +# $count has already been sanitized for acceptable values, so
> +# just collect $count bridges here.
> +for i in $(seq 0 $((count - 1))); do
> + bridge[$i]="$(bridge_filter "$i" <<< "$json")"
> +done
>
> # validate root ports per host bridge
> check_host_bridge()
>
> base-commit: 92d5203077553bfc9f7bf1c219563db0fc28e660
> prerequisite-patch-id: f17261693e3ac38880b7701a94a469bb513b4078
