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

[NDCTL PATCH v2] 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>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
v2:
- Merged Vishal's suggestion

 test/cxl-topology.sh | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
index 90b9c98273db..49e919a187af 100644
--- a/test/cxl-topology.sh
+++ b/test/cxl-topology.sh
@@ -37,15 +37,37 @@ 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_filter()
+{
+	local br_num="$1"
+
+	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'
+}
+
+# $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()
@@ -64,6 +86,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: 01eeaf2954b2c3ff52622d62fdae1c18cd15ab66
-- 
2.49.0

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

[Alison Schofield]

On Thu, May 08, 2025 at 01:44:19PM -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>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

Applied to pending, thanks!
https://github.com/pmem/ndctl/tree/pending


> ---
> v2:
> - Merged Vishal's suggestion
> 
>  test/cxl-topology.sh | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
> index 90b9c98273db..49e919a187af 100644
> --- a/test/cxl-topology.sh
> +++ b/test/cxl-topology.sh
> @@ -37,15 +37,37 @@ 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_filter()
> +{
> +	local br_num="$1"
> +
> +	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'
> +}
> +
> +# $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()
> @@ -64,6 +86,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: 01eeaf2954b2c3ff52622d62fdae1c18cd15ab66
> -- 
> 2.49.0
> 
> 

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

[Zhijian Li (Fujitsu)]

On 09/05/2025 04:44, 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>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

It seems it's a bit later, anyway

Tested-by: Li Zhijian <lizhijian@fujitsu.com>


> ---
> v2:
> - Merged Vishal's suggestion
> 
>   test/cxl-topology.sh | 31 +++++++++++++++++++++++++++----
>   1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
> index 90b9c98273db..49e919a187af 100644
> --- a/test/cxl-topology.sh
> +++ b/test/cxl-topology.sh
> @@ -37,15 +37,37 @@ 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_filter()
> +{
> +	local br_num="$1"
> +
> +	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'
> +}
> +
> +# $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()
> @@ -64,6 +86,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: 01eeaf2954b2c3ff52622d62fdae1c18cd15ab66

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

[Alison Schofield]

On Wed, May 14, 2025 at 06:58:45AM +0000, Zhijian Li (Fujitsu) wrote:
> 
> 
> On 09/05/2025 04:44, 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>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> 
> It seems it's a bit later, anyway
> 
> Tested-by: Li Zhijian <lizhijian@fujitsu.com>

Thanks! I'll pick up your tag if I rebase pending. (probably will ;))

> 
> 
> > ---
> > v2:
> > - Merged Vishal's suggestion
> > 
> >   test/cxl-topology.sh | 31 +++++++++++++++++++++++++++----
> >   1 file changed, 27 insertions(+), 4 deletions(-)
> > 
> > diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
> > index 90b9c98273db..49e919a187af 100644
> > --- a/test/cxl-topology.sh
> > +++ b/test/cxl-topology.sh
> > @@ -37,15 +37,37 @@ 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_filter()
> > +{
> > +	local br_num="$1"
> > +
> > +	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'
> > +}
> > +
> > +# $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()
> > @@ -64,6 +86,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: 01eeaf2954b2c3ff52622d62fdae1c18cd15ab66