[PATCH] Return single PathRecord for SubnAdmGet when SGID and/or DGID

[PATCH] Return single PathRecord for SubnAdmGet when SGID and/or DGID

[Eli Dorfman]

From: Eli Dorfman <elid-smomgflXvOZWk0Htik3J/w@public.gmane.org>

Signed-off-by: Eli Dorfman <elid-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_sa_path_record.c |   38 ++++++++++++++++++++++++-----------
 1 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/opensm/opensm/osm_sa_path_record.c b/opensm/opensm/osm_sa_path_record.c
index f36eb46..0c6621b 100644
--- a/opensm/opensm/osm_sa_path_record.c
+++ b/opensm/opensm/osm_sa_path_record.c
@@ -890,7 +890,7 @@ Exit:
 
 /**********************************************************************
  **********************************************************************/
-static void pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
+static int pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
 				       IN const osm_madw_t * p_madw,
 				       IN const osm_port_t * p_req_port,
 				       IN const osm_port_t * p_src_port,
@@ -908,7 +908,7 @@ static void pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
 	uint16_t dest_lid_max_ho;
 	uint16_t src_lid_ho;
 	uint16_t dest_lid_ho;
-	uint32_t path_num;
+	uint32_t path_num = 0;
 	uint8_t preference;
 	uintn_t iterations;
 	uintn_t src_offset;
@@ -1019,7 +1019,7 @@ static void pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
 	   Preferred paths come first in OpenSM
 	 */
 	preference = 0;
-	path_num = 0;
+	path_num = cl_qlist_count(p_list);
 
 	/* If SubnAdmGet, assume NumbPaths 1 (1.2 erratum) */
 	if (p_sa_mad->method != IB_MAD_METHOD_GET)
@@ -1111,6 +1111,7 @@ static void pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
 
 Exit:
 	OSM_LOG_EXIT(sa->p_log);
+	return path_num;
 }
 
 /**********************************************************************
@@ -1314,6 +1315,8 @@ static void pr_rcv_process_world(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 	const cl_qmap_t *p_tbl;
 	const osm_port_t *p_dest_port;
 	const osm_port_t *p_src_port;
+	const ib_sa_mad_t *p_sa_mad;
+	int   num_paths = 0;
 
 	OSM_LOG_ENTER(sa->p_log);
 
@@ -1326,14 +1329,17 @@ static void pr_rcv_process_world(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 	   any check to determine the reversability of the paths.
 	 */
 	p_tbl = &sa->p_subn->port_guid_tbl;
+	p_sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
 
 	p_dest_port = (osm_port_t *) cl_qmap_head(p_tbl);
 	while (p_dest_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
 		p_src_port = (osm_port_t *) cl_qmap_head(p_tbl);
 		while (p_src_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
-			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
-						   p_src_port, p_dest_port,
-						   p_dgid, comp_mask, p_list);
+			num_paths += pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
+								p_src_port, p_dest_port,
+								p_dgid, comp_mask, p_list);
+			if (p_sa_mad->method == IB_MAD_METHOD_GET && num_paths > 1)
+				return;
 
 			p_src_port =
 			    (osm_port_t *) cl_qmap_next(&p_src_port->map_item);
@@ -1358,6 +1364,8 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 {
 	const cl_qmap_t *p_tbl;
 	const osm_port_t *p_port;
+	const ib_sa_mad_t *p_sa_mad;
+	int   num_paths = 0;
 
 	OSM_LOG_ENTER(sa->p_log);
 
@@ -1367,6 +1375,7 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 	   need to special case that one.
 	 */
 	p_tbl = &sa->p_subn->port_guid_tbl;
+	p_sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
 
 	if (p_src_port) {
 		/*
@@ -1374,9 +1383,11 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 		 */
 		p_port = (osm_port_t *) cl_qmap_head(p_tbl);
 		while (p_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
-			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
-						   p_src_port, p_port, p_dgid,
-						   comp_mask, p_list);
+			num_paths += pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
+								p_src_port, p_port, p_dgid,
+								comp_mask, p_list);
+			if (p_sa_mad->method == IB_MAD_METHOD_GET && num_paths > 1)
+				goto Exit;
 			p_port = (osm_port_t *) cl_qmap_next(&p_port->map_item);
 		}
 	} else {
@@ -1385,13 +1396,16 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 		 */
 		p_port = (osm_port_t *) cl_qmap_head(p_tbl);
 		while (p_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
-			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
-						   p_port, p_dest_port, p_dgid,
-						   comp_mask, p_list);
+			num_paths += pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
+								p_port, p_dest_port, p_dgid,
+								comp_mask, p_list);
+			if (p_sa_mad->method == IB_MAD_METHOD_GET && num_paths > 1)
+				goto Exit;
 			p_port = (osm_port_t *) cl_qmap_next(&p_port->map_item);
 		}
 	}
 
+Exit:
 	OSM_LOG_EXIT(sa->p_log);
 }
 
-- 
1.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Return single PathRecord for SubnAdmGet when SGID and/or DGID

[Sasha Khapyorsky]

Hi Eli,

On 13:40 Fri 30 Oct     , Eli Dorfman wrote:
> From: Eli Dorfman <elid-smomgflXvOZWk0Htik3J/w@public.gmane.org>

Please add descriptive change log. It is hard (for me) to just remember
an issue in all details.

> 
> Signed-off-by: Eli Dorfman <elid-smomgflXvOZWk0Htik3J/w@public.gmane.org>
> ---
>  opensm/opensm/osm_sa_path_record.c |   38 ++++++++++++++++++++++++-----------
>  1 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/opensm/opensm/osm_sa_path_record.c b/opensm/opensm/osm_sa_path_record.c
> index f36eb46..0c6621b 100644
> --- a/opensm/opensm/osm_sa_path_record.c
> +++ b/opensm/opensm/osm_sa_path_record.c
> @@ -890,7 +890,7 @@ Exit:
>  
>  /**********************************************************************
>   **********************************************************************/
> -static void pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
> +static int pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
>  				       IN const osm_madw_t * p_madw,
>  				       IN const osm_port_t * p_req_port,
>  				       IN const osm_port_t * p_src_port,
> @@ -908,7 +908,7 @@ static void pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
>  	uint16_t dest_lid_max_ho;
>  	uint16_t src_lid_ho;
>  	uint16_t dest_lid_ho;
> -	uint32_t path_num;
> +	uint32_t path_num = 0;

It is reinitialized later as: 

	path_num = cl_qlist_count(p_list);

, one of them is not needed.

>  	uint8_t preference;
>  	uintn_t iterations;
>  	uintn_t src_offset;
> @@ -1019,7 +1019,7 @@ static void pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
>  	   Preferred paths come first in OpenSM
>  	 */
>  	preference = 0;
> -	path_num = 0;
> +	path_num = cl_qlist_count(p_list);

Is this correct?

In this way pr_rcv_get_port_pair_paths() will return a total number of
PRs collected in previous calls too (not for just specific
source/destination call). No?

>  
>  	/* If SubnAdmGet, assume NumbPaths 1 (1.2 erratum) */
>  	if (p_sa_mad->method != IB_MAD_METHOD_GET)
> @@ -1111,6 +1111,7 @@ static void pr_rcv_get_port_pair_paths(IN osm_sa_t * sa,
>  
>  Exit:
>  	OSM_LOG_EXIT(sa->p_log);
> +	return path_num;
>  }
>  
>  /**********************************************************************
> @@ -1314,6 +1315,8 @@ static void pr_rcv_process_world(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
>  	const cl_qmap_t *p_tbl;
>  	const osm_port_t *p_dest_port;
>  	const osm_port_t *p_src_port;
> +	const ib_sa_mad_t *p_sa_mad;
> +	int   num_paths = 0;
>  
>  	OSM_LOG_ENTER(sa->p_log);
>  
> @@ -1326,14 +1329,17 @@ static void pr_rcv_process_world(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
>  	   any check to determine the reversability of the paths.
>  	 */
>  	p_tbl = &sa->p_subn->port_guid_tbl;
> +	p_sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
>  
>  	p_dest_port = (osm_port_t *) cl_qmap_head(p_tbl);
>  	while (p_dest_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
>  		p_src_port = (osm_port_t *) cl_qmap_head(p_tbl);
>  		while (p_src_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
> -			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
> -						   p_src_port, p_dest_port,
> -						   p_dgid, comp_mask, p_list);
> +			num_paths += pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
> +								p_src_port, p_dest_port,
> +								p_dgid, comp_mask, p_list);
> +			if (p_sa_mad->method == IB_MAD_METHOD_GET && num_paths > 1)
> +				return;

So it will return with num_paths > 1. Then wouldn't an error (too many
records) be generated by osm_sa_respond() (just similar to as it is
now)? I guess so.

So shouldn't here be something like:

	if (p_sa_mad->method == IB_MAD_METHOD_GET &&
	    cl_qlist_count(p_list) >= 1)
		break;

(, and then you don't need to bother with num_paths in
pr_rcv_get_port_pair_paths())?

>  
>  			p_src_port =
>  			    (osm_port_t *) cl_qmap_next(&p_src_port->map_item);
> @@ -1358,6 +1364,8 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
>  {
>  	const cl_qmap_t *p_tbl;
>  	const osm_port_t *p_port;
> +	const ib_sa_mad_t *p_sa_mad;
> +	int   num_paths = 0;
>  
>  	OSM_LOG_ENTER(sa->p_log);
>  
> @@ -1367,6 +1375,7 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
>  	   need to special case that one.
>  	 */
>  	p_tbl = &sa->p_subn->port_guid_tbl;
> +	p_sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
>  
>  	if (p_src_port) {
>  		/*
> @@ -1374,9 +1383,11 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
>  		 */
>  		p_port = (osm_port_t *) cl_qmap_head(p_tbl);
>  		while (p_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
> -			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
> -						   p_src_port, p_port, p_dgid,
> -						   comp_mask, p_list);
> +			num_paths += pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
> +								p_src_port, p_port, p_dgid,
> +								comp_mask, p_list);
> +			if (p_sa_mad->method == IB_MAD_METHOD_GET && num_paths > 1)

Ditto.

> +				goto Exit;

'break' will work too.

>  			p_port = (osm_port_t *) cl_qmap_next(&p_port->map_item);
>  		}
>  	} else {
> @@ -1385,13 +1396,16 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
>  		 */
>  		p_port = (osm_port_t *) cl_qmap_head(p_tbl);
>  		while (p_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
> -			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
> -						   p_port, p_dest_port, p_dgid,
> -						   comp_mask, p_list);
> +			num_paths += pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
> +								p_port, p_dest_port, p_dgid,
> +								comp_mask, p_list);
> +			if (p_sa_mad->method == IB_MAD_METHOD_GET && num_paths > 1)

Ditto.

> +				goto Exit;

'break' will work too

>  			p_port = (osm_port_t *) cl_qmap_next(&p_port->map_item);
>  		}
>  	}
>  
> +Exit:

and then exit label is not needed.

Sasha

>  	OSM_LOG_EXIT(sa->p_log);
>  }
>  
> -- 
> 1.5.5
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] opensm: Return single PathRecord for SubnAdmGet with DGID/SGID wild carded

[elid]

Return single PathRecord for SubnAdmGet with DGID/SGID wildcarded

Instead of iterating over all ports in the fabric and returning an error (TOO_MANY_RECORDS),
when SGID and/or DGID are wild carded return only single PathRecord.

Signed-off-by: Eli Dorfman <elid-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_sa_path_record.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/opensm/opensm/osm_sa_path_record.c b/opensm/opensm/osm_sa_path_record.c
index c2ef8c5..b3e1072 100644
--- a/opensm/opensm/osm_sa_path_record.c
+++ b/opensm/opensm/osm_sa_path_record.c
@@ -1313,6 +1313,7 @@ static void pr_rcv_process_world(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 	const cl_qmap_t *p_tbl;
 	const osm_port_t *p_dest_port;
 	const osm_port_t *p_src_port;
+	const ib_sa_mad_t *p_sa_mad;
 
 	OSM_LOG_ENTER(sa->p_log);
 
@@ -1325,6 +1326,7 @@ static void pr_rcv_process_world(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 	   any check to determine the reversability of the paths.
 	 */
 	p_tbl = &sa->p_subn->port_guid_tbl;
+	p_sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
 
 	p_dest_port = (osm_port_t *) cl_qmap_head(p_tbl);
 	while (p_dest_port != (osm_port_t *) cl_qmap_end(p_tbl)) {
@@ -1333,6 +1335,9 @@ static void pr_rcv_process_world(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
 						   p_src_port, p_dest_port,
 						   p_dgid, comp_mask, p_list);
+			if (p_sa_mad->method == IB_MAD_METHOD_GET && 
+			    cl_qlist_count(p_list) > 0)
+				goto Exit;
 
 			p_src_port =
 			    (osm_port_t *) cl_qmap_next(&p_src_port->map_item);
@@ -1342,6 +1347,7 @@ static void pr_rcv_process_world(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 		    (osm_port_t *) cl_qmap_next(&p_dest_port->map_item);
 	}
 
+Exit:
 	OSM_LOG_EXIT(sa->p_log);
 }
 
@@ -1357,6 +1363,7 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 {
 	const cl_qmap_t *p_tbl;
 	const osm_port_t *p_port;
+	const ib_sa_mad_t *p_sa_mad;
 
 	OSM_LOG_ENTER(sa->p_log);
 
@@ -1366,6 +1373,7 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 	   need to special case that one.
 	 */
 	p_tbl = &sa->p_subn->port_guid_tbl;
+	p_sa_mad = osm_madw_get_sa_mad_ptr(p_madw);
 
 	if (p_src_port) {
 		/*
@@ -1376,6 +1384,9 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
 						   p_src_port, p_port, p_dgid,
 						   comp_mask, p_list);
+			if (p_sa_mad->method == IB_MAD_METHOD_GET && 
+			    cl_qlist_count(p_list) > 0)
+				break;
 			p_port = (osm_port_t *) cl_qmap_next(&p_port->map_item);
 		}
 	} else {
@@ -1387,6 +1398,9 @@ static void pr_rcv_process_half(IN osm_sa_t * sa, IN const osm_madw_t * p_madw,
 			pr_rcv_get_port_pair_paths(sa, p_madw, requester_port,
 						   p_port, p_dest_port, p_dgid,
 						   comp_mask, p_list);
+			if (p_sa_mad->method == IB_MAD_METHOD_GET && 
+			    cl_qlist_count(p_list) > 0)
+				break;
 			p_port = (osm_port_t *) cl_qmap_next(&p_port->map_item);
 		}
 	}
-- 
1.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] opensm: Return single PathRecord for SubnAdmGet with DGID/SGID wild carded

[Sasha Khapyorsky]

On 17:59 Mon 02 Nov     , elid wrote:
> Return single PathRecord for SubnAdmGet with DGID/SGID wildcarded
> 
> Instead of iterating over all ports in the fabric and returning an error (TOO_MANY_RECORDS),
> when SGID and/or DGID are wild carded return only single PathRecord.
> 
> Signed-off-by: Eli Dorfman <elid-smomgflXvOZWk0Htik3J/w@public.gmane.org>

Applied. Thanks.

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html