[NDCTL PATCH] cxl: Change cxl-topology.sh assumption on host bridge validation

[NDCTL PATCH] cxl: Change cxl-topology.sh assumption on host bridge validation

[Dave Jiang]

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.jiang@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")
 
 # validate root ports per host bridge
 check_host_bridge()
@@ -64,6 +65,7 @@ json=$($CXL list -b cxl_test -P -p ${bridge[0]})
 count=$(jq ".[] | .[\"ports:${bridge[0]}\"] | length" <<< $json)
 ((count == 2)) || err "$LINENO"
 
+port_sort="sort_by(.port | .[4:] | tonumber)"
 switch[0]=$(jq -r ".[] | .[\"ports:${bridge[0]}\"] | $port_sort | .[0].host" <<< $json)
 switch[1]=$(jq -r ".[] | .[\"ports:${bridge[0]}\"] | $port_sort | .[1].host" <<< $json)
 

base-commit: 92d5203077553bfc9f7bf1c219563db0fc28e660
prerequisite-patch-id: b16d7a7db948e38a7752c12bdaa34116b5967e00
prerequisite-patch-id: 35769202bdba1fed259c072ca7ef279c075131e7
prerequisite-patch-id: b85fc29353224d045e8793c99829f4b372095629
-- 
2.49.0

Re: [NDCTL PATCH] cxl: Change cxl-topology.sh assumption on host bridge validation

[Verma, Vishal L]

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.jiang@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):

-->8--

From 996610fac0a751acc5d68a56ff7d6746348a80c4 Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.verma@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.verma@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
-- 
2.49.0

Re: [NDCTL PATCH] cxl: Change cxl-topology.sh assumption on host bridge validation

[Dave Jiang]

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.jiang@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.verma@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.verma@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