public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
@ 2009-12-01 19:41 Hal Rosenstock
       [not found] ` <20091201194110.GA26753-Wuw85uim5zDR7s880joybQ@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2009-12-01 19:41 UTC (permalink / raw)
  To: sashak-smomgflXvOZWk0Htik3J/w; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA


Optimized SLtoVLMappingTable programming reduces the number of MADs
needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
SLtoVLMappingTable.

Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Changes since v2:
Use osm_log rather than OSM_LOG in osm_slvl_rcv_process for optimized
sl2vl mapping programming verbose log message
Removed unneeded check of p_physp in input port loop in osm_slvl_rcv_process
for optimized sl2vl mapping programming

Changes since v1:
Changed optimized programming flow in osm_qos to only call
sl2vl_update once per switch supporting this feature

diff --git a/opensm/include/opensm/osm_subnet.h b/opensm/include/opensm/osm_subnet.h
index 3c08689..583c070 100644
--- a/opensm/include/opensm/osm_subnet.h
+++ b/opensm/include/opensm/osm_subnet.h
@@ -206,6 +206,7 @@ typedef struct osm_subn_opt {
 	boolean_t daemon;
 	boolean_t sm_inactive;
 	boolean_t babbling_port_policy;
+	boolean_t use_optimized_slvl;
 	osm_qos_options_t qos_options;
 	osm_qos_options_t qos_ca_options;
 	osm_qos_options_t qos_sw0_options;
@@ -433,6 +434,10 @@ typedef struct osm_subn_opt {
 *	babbling_port_policy
 *		OpenSM will enforce its "babbling" port policy.
 *
+*	use_optimized_slvl
+*		Use optimized SLtoVLMappingTable programming if
+*		device indicates it supports this.
+*
 *	perfmgr
 *		Enable or disable the performance manager
 *
diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
index 08f9a60..617a86e 100644
--- a/opensm/opensm/osm_qos.c
+++ b/opensm/opensm/osm_qos.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2006-2009 Voltaire, Inc. All rights reserved.
+ * Copyright (c) 2009 HNR Consulting. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -150,7 +151,7 @@ static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
 
 static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
 					  uint8_t in_port, uint8_t out_port,
-					  unsigned force_update,
+					  unsigned optimize, unsigned force_update,
 					  const ib_slvl_table_t * sl2vl_table)
 {
 	osm_madw_context_t context;
@@ -180,41 +181,47 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
 	context.slvl_context.node_guid = osm_node_get_node_guid(p_node);
 	context.slvl_context.port_guid = osm_physp_get_port_guid(p);
 	context.slvl_context.set_method = TRUE;
-	attr_mod = in_port << 8 | out_port;
+	if (optimize)
+		/* wildcard both input and output ports */
+		attr_mod = 0x30000;
+	else
+		attr_mod = in_port << 8 | out_port;
 	return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
 			   (uint8_t *) & tbl, sizeof(tbl),
 			   IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
 			   CL_DISP_MSGID_NONE, &context);
 }
 
-static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
+static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_physp_t * p0,
 				    osm_physp_t * p, uint8_t port_num,
-				    unsigned force_update,
+				    unsigned optimize, unsigned force_update,
 				    const struct qos_config *qcfg)
 {
 	ib_api_status_t status;
 	uint8_t i, num_ports;
-	osm_physp_t *p_physp;
+	osm_node_t *p_node;
 
-	if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH) {
+	p_node = osm_physp_get_node_ptr(p);
+	if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
 		if (ib_port_info_get_vl_cap(&p->port_info) == 1) {
 			/* Check port 0's capability mask */
-			p_physp = p_port->p_physp;
-			if (!
-			    (p_physp->port_info.
-			     capability_mask & IB_PORT_CAP_HAS_SL_MAP))
+			if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
 				return IB_SUCCESS;
 		}
-		num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
+		num_ports = osm_node_get_num_physp(p_node);
 	} else {
 		if (!(p->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
 			return IB_SUCCESS;
 		num_ports = 1;
 	}
 
+	if (optimize)
+		return sl2vl_update_table(sm, p, 1, port_num, optimize,
+					  force_update, &qcfg->sl2vl);
+
 	for (i = 0; i < num_ports; i++) {
-		status = sl2vl_update_table(sm, p, i, port_num, force_update,
-					    &qcfg->sl2vl);
+		status = sl2vl_update_table(sm, p, i, port_num, optimize,
+					    force_update, &qcfg->sl2vl);
 		if (status != IB_SUCCESS)
 			return status;
 	}
@@ -222,22 +229,30 @@ static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
 	return IB_SUCCESS;
 }
 
-static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
-			   osm_port_t * p_port, osm_physp_t * p,
-			   uint8_t port_num, unsigned force_update,
-			   const struct qos_config *qcfg)
+static int vlarb_physp_setup(osm_sm_t * sm, osm_physp_t * p, uint8_t port_num,
+			     unsigned force_update,
+			     const struct qos_config *qcfg)
 {
-	ib_api_status_t status;
-
 	/* OpVLs should be ok at this moment - just use it */
 
 	/* setup VL high limit on the physp later to be updated by link mgr */
 	p->vl_high_limit = qcfg->vl_high_limit;
 
 	/* setup VLArbitration */
-	status = vlarb_update(sm, p, port_num, force_update, qcfg);
+	return vlarb_update(sm, p, port_num, force_update, qcfg);
+}
+
+static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
+			   osm_physp_t * p0, osm_physp_t * p,
+			   uint8_t port_num, unsigned force_update,
+			   const struct qos_config *qcfg)
+{
+	ib_api_status_t status;
+
+	/* setup VLArbitration */
+	status = vlarb_physp_setup(sm, p, port_num, force_update, qcfg);
 	if (status != IB_SUCCESS) {
-		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6202 : "
+		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6202: "
 			"failed to update VLArbitration tables "
 			"for port %" PRIx64 " #%d\n",
 			cl_ntoh64(p->port_guid), port_num);
@@ -245,9 +260,9 @@ static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
 	}
 
 	/* setup SL2VL tables */
-	status = sl2vl_update(sm, p_port, p, port_num, force_update, qcfg);
+	status = sl2vl_update(sm, p0, p, port_num, 0, force_update, qcfg);
 	if (status != IB_SUCCESS) {
-		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6203 : "
+		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6203: "
 			"failed to update SL2VLMapping tables "
 			"for port %" PRIx64 " #%d\n",
 			cl_ntoh64(p->port_guid), port_num);
@@ -267,6 +282,7 @@ int osm_qos_setup(osm_opensm_t * p_osm)
 	uint32_t num_physp;
 	osm_physp_t *p_physp;
 	osm_node_t *p_node;
+	osm_switch_t *p_sw;
 	unsigned force_update;
 	int ret = 0;
 	uint8_t i;
@@ -290,6 +306,44 @@ int osm_qos_setup(osm_opensm_t * p_osm)
 	/* read QoS policy config file */
 	osm_qos_parse_policy_file(&p_osm->subn);
 
+	/* loop on switches that support optimized SL2VL programming first */
+	p_tbl = &p_osm->subn.sw_guid_tbl;
+	p_next = cl_qmap_head(p_tbl);
+	while (p_next != cl_qmap_end(p_tbl)) {
+		p_sw = (osm_switch_t *) p_next;
+		p_next = cl_qmap_next(p_next);
+
+		if (ib_switch_info_get_opt_sl2vlmapping(&p_sw->switch_info) &&
+		    p_osm->subn.opt.use_optimized_slvl) {
+			p_physp = osm_node_get_physp_ptr(p_sw->p_node, 1);
+			num_physp = osm_node_get_num_physp(p_sw->p_node);
+			force_update = p_osm->subn.need_update;
+			for (i = 1; i < num_physp; i++) {
+				p_physp = osm_node_get_physp_ptr(p_sw->p_node, i);
+				if (!p_physp)
+					continue;
+				if (vlarb_physp_setup(&p_osm->sm, p_physp, i,
+						      p_physp->need_update ||
+						      p_osm->subn.need_update,
+						      &swe_config))
+					ret = -1;
+				force_update |= p_physp->need_update;
+			}
+			if (sl2vl_update(&p_osm->sm,
+					 osm_node_get_physp_ptr(p_sw->p_node, 0),
+					 p_physp, i, 1, force_update,
+					 &swe_config)) {
+				OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 6204: "
+					"failed to update optimized SL2VLMapping"
+					" tables for port %" PRIx64 " #%d\n",
+					cl_ntoh64(p_physp->port_guid), i);
+				ret = -1;
+			}
+		}
+	}
+
+	/* now, loop on ports skipping the external ports of switches
+	   that support optimized SL2VL programming */
 	p_tbl = &p_osm->subn.port_guid_tbl;
 	p_next = cl_qmap_head(p_tbl);
 	while (p_next != cl_qmap_end(p_tbl)) {
@@ -298,6 +352,11 @@ int osm_qos_setup(osm_opensm_t * p_osm)
 
 		p_node = p_port->p_node;
 		if (p_node->sw) {
+			/* skip switches with optimized SL2VL mapping
+			   programming since already done */
+			if (ib_switch_info_get_opt_sl2vlmapping(&p_node->sw->switch_info) &&
+			    p_osm->subn.opt.use_optimized_slvl)
+				goto check_port0;
 			num_physp = osm_node_get_num_physp(p_node);
 			for (i = 1; i < num_physp; i++) {
 				p_physp = osm_node_get_physp_ptr(p_node, i);
@@ -306,10 +365,11 @@ int osm_qos_setup(osm_opensm_t * p_osm)
 				force_update = p_physp->need_update ||
 				    p_osm->subn.need_update;
 				if (qos_physp_setup(&p_osm->log, &p_osm->sm,
-						    p_port, p_physp, i,
+						    p_port->p_physp, p_physp, i,
 						    force_update, &swe_config))
 					ret = -1;
 			}
+check_port0:
 			/* skip base port 0 */
 			if (!ib_switch_info_is_enhanced_port0
 			    (&p_node->sw->switch_info))
@@ -326,8 +386,8 @@ int osm_qos_setup(osm_opensm_t * p_osm)
 			continue;
 
 		force_update = p_physp->need_update || p_osm->subn.need_update;
-		if (qos_physp_setup(&p_osm->log, &p_osm->sm, p_port, p_physp,
-				    0, force_update, cfg))
+		if (qos_physp_setup(&p_osm->log, &p_osm->sm, p_port->p_physp,
+				    p_physp, 0, force_update, cfg))
 			ret = -1;
 	}
 
diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
index 4f75690..214a763 100644
--- a/opensm/opensm/osm_slvl_map_rcv.c
+++ b/opensm/opensm/osm_slvl_map_rcv.c
@@ -2,6 +2,7 @@
  * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
  * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
  * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
+ * Copyright (c) 2009 HNR Consulting. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -70,7 +71,9 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 	osm_slvl_context_t *p_context;
 	ib_net64_t port_guid;
 	ib_net64_t node_guid;
-	uint8_t out_port_num, in_port_num;
+	uint32_t attr_mod;
+	uint8_t out_port_num, in_port_num, startinport, startoutport,
+		endinport, endoutport;
 
 	CL_ASSERT(sm);
 
@@ -109,6 +112,9 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 		    (uint8_t) cl_ntoh32(p_smp->attr_mod & 0xFF000000);
 		in_port_num =
 		    (uint8_t) cl_ntoh32((p_smp->attr_mod & 0x00FF0000) << 8);
+		attr_mod = cl_ntoh32(p_smp->attr_mod);
+		if (attr_mod & 0x30000)
+			goto opt_sl2vl;
 		p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
 	} else {
 		p_physp = p_port->p_physp;
@@ -121,7 +127,7 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 	   all we want is to update the subnet.
 	 */
 	OSM_LOG(sm->p_log, OSM_LOG_VERBOSE,
-		"Got SLtoVL get response in_port_num %u out_port_num %u with "
+		"Received SLtoVL GetResp in_port_num %u out_port_num %u with "
 		"GUID 0x%" PRIx64 " for parent node GUID 0x%" PRIx64 ", TID 0x%"
 		PRIx64 "\n", in_port_num, out_port_num, cl_ntoh64(port_guid),
 		cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
@@ -140,6 +146,38 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 				out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
 
 	osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
+	goto Exit;
+
+opt_sl2vl:
+	if (osm_log_is_active(sm->p_log, OSM_LOG_VERBOSE))
+		osm_log(sm->p_log, OSM_LOG_VERBOSE,
+			"Received optimized SLtoVL get response in_port_num %u "
+			"out_port_num %u with GUID 0x%" PRIx64 " for parent "
+			"node GUID 0x%" PRIx64 ", TID 0x%" PRIx64 "\n",
+			in_port_num, out_port_num, cl_ntoh64(port_guid),
+			cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
+
+	osm_dump_slvl_map_table(sm->p_log, port_guid, in_port_num,
+				out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
+
+	if (attr_mod & 0x10000) {
+		startoutport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
+		endoutport = osm_node_get_num_physp(p_node);
+	} else
+		endoutport = startoutport = out_port_num;
+	if (attr_mod & 0x20000) {
+		startinport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
+		endinport = osm_node_get_num_physp(p_node);
+	} else
+		endinport = startinport = in_port_num;
+
+	for (out_port_num = startoutport; out_port_num < endoutport;
+	     out_port_num++) {
+		p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
+		for (in_port_num = startinport; in_port_num < endinport;
+		     in_port_num++)
+			osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
+	}
 
 Exit:
 	cl_plock_release(sm->p_lock);
diff --git a/opensm/opensm/osm_subnet.c b/opensm/opensm/osm_subnet.c
index 09029db..103b5c6 100644
--- a/opensm/opensm/osm_subnet.c
+++ b/opensm/opensm/osm_subnet.c
@@ -354,6 +354,7 @@ static const opt_rec_t opt_tbl[] = {
 	{ "daemon", OPT_OFFSET(daemon), opts_parse_boolean, NULL, 0 },
 	{ "sm_inactive", OPT_OFFSET(sm_inactive), opts_parse_boolean, NULL, 1 },
 	{ "babbling_port_policy", OPT_OFFSET(babbling_port_policy), opts_parse_boolean, NULL, 1 },
+	{ "use_optimized_slvl", OPT_OFFSET(use_optimized_slvl), opts_parse_boolean, NULL, 1 },
 #ifdef ENABLE_OSM_PERF_MGR
 	{ "perfmgr", OPT_OFFSET(perfmgr), opts_parse_boolean, NULL, 0 },
 	{ "perfmgr_redir", OPT_OFFSET(perfmgr_redir), opts_parse_boolean, NULL, 0 },
@@ -713,6 +714,7 @@ void osm_subn_set_default_opt(IN osm_subn_opt_t * p_opt)
 	p_opt->daemon = FALSE;
 	p_opt->sm_inactive = FALSE;
 	p_opt->babbling_port_policy = FALSE;
+	p_opt->use_optimized_slvl = FALSE;
 #ifdef ENABLE_OSM_PERF_MGR
 	p_opt->perfmgr = FALSE;
 	p_opt->perfmgr_redir = TRUE;
@@ -1499,10 +1501,13 @@ int osm_subn_output_conf(FILE *out, IN osm_subn_opt_t * p_opts)
 		"# SM Inactive\n"
 		"sm_inactive %s\n\n"
 		"# Babbling Port Policy\n"
-		"babbling_port_policy %s\n\n",
+		"babbling_port_policy %s\n\n"
+		"# Use Optimized SLtoVLMapping programming if supported by device\n"
+		"use_optimized_slvl %s\n\n",
 		p_opts->daemon ? "TRUE" : "FALSE",
 		p_opts->sm_inactive ? "TRUE" : "FALSE",
-		p_opts->babbling_port_policy ? "TRUE" : "FALSE");
+		p_opts->babbling_port_policy ? "TRUE" : "FALSE",
+		p_opts->use_optimized_slvl ? "TRUE" : "FALSE");
 
 #ifdef ENABLE_OSM_PERF_MGR
 	fprintf(out,
--
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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH] opensm/osm_slvl_map_rcv.c: fix port parsing on BE machine
       [not found] ` <20091201194110.GA26753-Wuw85uim5zDR7s880joybQ@public.gmane.org>
@ 2009-12-29 15:05   ` Sasha Khapyorsky
  2009-12-29 15:07     ` [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network Sasha Khapyorsky
  2009-12-29 15:06   ` [PATCH] opensm/osm_slvl_map_rcv.c: fix mutex double release bug Sasha Khapyorsky
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2009-12-29 15:05 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA


Ports (input and output) number parsing from attribute modifier value of
SLtoVLMappingTable response was endianess dependent and actually was
broken on a big endian machines. Fix it by doing this endianess
independently.

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---

On 14:41 Tue 01 Dec     , Hal Rosenstock wrote:
> 
> Optimized SLtoVLMappingTable programming reduces the number of MADs
> needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
> SLtoVLMappingTable.
> 
> Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

When reviewing this patch I found couple of bugs in this code unrelated
to the proposed change. Posting the fixes in this thread.

 opensm/opensm/osm_slvl_map_rcv.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
index 4f75690..3b24a8e 100644
--- a/opensm/opensm/osm_slvl_map_rcv.c
+++ b/opensm/opensm/osm_slvl_map_rcv.c
@@ -105,10 +105,8 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 
 	/* in case of a non switch node the attr modifier should be ignored */
 	if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
-		out_port_num =
-		    (uint8_t) cl_ntoh32(p_smp->attr_mod & 0xFF000000);
-		in_port_num =
-		    (uint8_t) cl_ntoh32((p_smp->attr_mod & 0x00FF0000) << 8);
+		out_port_num = cl_ntoh32(p_smp->attr_mod) & 0xff;
+		in_port_num = (cl_ntoh32(p_smp->attr_mod) >> 8) & 0xff;
 		p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
 	} else {
 		p_physp = p_port->p_physp;
-- 
1.6.6.rc4

--
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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH] opensm/osm_slvl_map_rcv.c: fix mutex double release bug
       [not found] ` <20091201194110.GA26753-Wuw85uim5zDR7s880joybQ@public.gmane.org>
  2009-12-29 15:05   ` [PATCH] opensm/osm_slvl_map_rcv.c: fix port parsing on BE machine Sasha Khapyorsky
@ 2009-12-29 15:06   ` Sasha Khapyorsky
  2009-12-29 16:29   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 37+ messages in thread
From: Sasha Khapyorsky @ 2009-12-29 15:06 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Hal Rosenstock


In case when response was received for non-existing port (for example
when it was dropped after querying) OpenSM data structures access mutex
will be released twice. Fix this.

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_slvl_map_rcv.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
index 3b24a8e..b35d867 100644
--- a/opensm/opensm/osm_slvl_map_rcv.c
+++ b/opensm/opensm/osm_slvl_map_rcv.c
@@ -91,7 +91,6 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 	p_port = osm_get_port_by_guid(sm->p_subn, port_guid);
 
 	if (!p_port) {
-		cl_plock_release(sm->p_lock);
 		OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 2C06: "
 			"No port object for port with GUID 0x%" PRIx64
 			"\n\t\t\t\tfor parent node GUID 0x%" PRIx64
-- 
1.6.6.rc4

--
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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
  2009-12-29 15:05   ` [PATCH] opensm/osm_slvl_map_rcv.c: fix port parsing on BE machine Sasha Khapyorsky
@ 2009-12-29 15:07     ` Sasha Khapyorsky
  2009-12-30 16:09       ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2009-12-29 15:07 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Hal Rosenstock


Verify that port number values received from network are in range and
valid for access. Report error otherwise.

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_slvl_map_rcv.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
index b35d867..6229db9 100644
--- a/opensm/opensm/osm_slvl_map_rcv.c
+++ b/opensm/opensm/osm_slvl_map_rcv.c
@@ -104,8 +104,17 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 
 	/* in case of a non switch node the attr modifier should be ignored */
 	if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
+		unsigned num_ports = osm_node_get_num_physp(p_node) - 1;
 		out_port_num = cl_ntoh32(p_smp->attr_mod) & 0xff;
 		in_port_num = (cl_ntoh32(p_smp->attr_mod) >> 8) & 0xff;
+		if (in_port_num > num_ports || out_port_num > num_ports) {
+			OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 2C07"
+				"Invalid attribute modifier 0x%x reveived in"
+				" response from switch 0x%" PRIx64 "\n",
+				cl_ntoh32(p_smp->attr_mod),
+				cl_ntoh64(node_guid));
+			goto Exit;
+		}
 		p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
 	} else {
 		p_physp = p_port->p_physp;
-- 
1.6.6.rc4

--
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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
       [not found] ` <20091201194110.GA26753-Wuw85uim5zDR7s880joybQ@public.gmane.org>
  2009-12-29 15:05   ` [PATCH] opensm/osm_slvl_map_rcv.c: fix port parsing on BE machine Sasha Khapyorsky
  2009-12-29 15:06   ` [PATCH] opensm/osm_slvl_map_rcv.c: fix mutex double release bug Sasha Khapyorsky
@ 2009-12-29 16:29   ` Sasha Khapyorsky
  2009-12-30 16:08     ` Hal Rosenstock
  2010-01-04 17:01   ` [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check Sasha Khapyorsky
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2009-12-29 16:29 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Hi Hal,

On 14:41 Tue 01 Dec     , Hal Rosenstock wrote:
> 
> Optimized SLtoVLMappingTable programming reduces the number of MADs
> needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
> SLtoVLMappingTable.
> 
> Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Changes since v2:
> Use osm_log rather than OSM_LOG in osm_slvl_rcv_process for optimized
> sl2vl mapping programming verbose log message

I didn't ask for such change. Instead I asked to avoid the function
(osm_slvl_rcv_process() in this patch) constructed in form of:

	if (cond1)
		goto flow1;

	do_flow2;

	goto end;

flow1:
	do_flow1;

end:

In general this is equivalent to simpler and more readable:

	if (cond1)
		do_flow1();
	else
		do_flow2();

And in case of osm_slvl_rcv_process() even nicer merge is possible when
slvl port table setup loop is shared, but its range (start*port and
end*port values) is defined differently for various input cases. Just
like this (against your patch, rebased over recent fixes, compilation
only tested):


diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
index 7ca0b8f..73c6253 100644
--- a/opensm/opensm/osm_slvl_map_rcv.c
+++ b/opensm/opensm/osm_slvl_map_rcv.c
@@ -72,8 +72,8 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 	ib_net64_t port_guid;
 	ib_net64_t node_guid;
 	uint32_t attr_mod;
-	uint8_t out_port_num, in_port_num, startinport, startoutport,
-		endinport, endoutport;
+	uint8_t startinport, endinport, startoutport, endoutport;
+	uint8_t in_port, out_port;
 
 	CL_ASSERT(sm);
 
@@ -109,80 +109,51 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
 	if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
 		unsigned num_ports = osm_node_get_num_physp(p_node) - 1;
 		attr_mod = cl_ntoh32(p_smp->attr_mod);
-		out_port_num = attr_mod & 0xff;
-		in_port_num = (attr_mod >> 8) & 0xff;
-		if (in_port_num > num_ports || out_port_num > num_ports) {
+
+		if (attr_mod & 0x10000) {
+			startoutport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
+			endoutport = osm_node_get_num_physp(p_node) - 1;
+		} else
+			startoutport = endoutport = attr_mod & 0xff;
+
+		if (attr_mod & 0x20000) {
+			startinport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
+			endinport = osm_node_get_num_physp(p_node) - 1;
+		} else
+			startinport = endinport = (attr_mod >> 8) & 0xff;
+
+		if (startinport > num_ports || startoutport > num_ports) {
 			OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 2C07"
 				"Invalid attribute modifier 0x%x reveived in"
 				" response from switch 0x%" PRIx64 "\n",
-				cl_ntoh32(p_smp->attr_mod),
-				cl_ntoh64(node_guid));
+				cl_ntoh32(attr_mod), cl_ntoh64(node_guid));
 			goto Exit;
 		}
-		if (attr_mod & 0x30000)
-			goto opt_sl2vl;
-		p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
-	} else {
-		p_physp = p_port->p_physp;
-		out_port_num = p_physp->port_num;
-		in_port_num = 0;
-	}
 
-	/*
-	   We do not care if this is a result of a set or get -
-	   all we want is to update the subnet.
-	 */
-	OSM_LOG(sm->p_log, OSM_LOG_VERBOSE,
-		"Received SLtoVL GetResp in_port_num %u out_port_num %u with "
-		"GUID 0x%" PRIx64 " for parent node GUID 0x%" PRIx64 ", TID 0x%"
-		PRIx64 "\n", in_port_num, out_port_num, cl_ntoh64(port_guid),
-		cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
-
-	/*
-	   Determine if we encountered a new Physical Port.
-	   If so, Ignore it.
-	 */
-	if (!p_physp) {
-		OSM_LOG(sm->p_log, OSM_LOG_ERROR,
-			"Got invalid port number %u\n", out_port_num);
-		goto Exit;
+	} else {
+		startoutport = endoutport = p_port->p_physp->port_num;
+		startinport = endinport = 0;
 	}
 
-	osm_dump_slvl_map_table(sm->p_log, port_guid, in_port_num,
-				out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
-
-	osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
-	goto Exit;
-
-opt_sl2vl:
-	if (osm_log_is_active(sm->p_log, OSM_LOG_VERBOSE))
-		osm_log(sm->p_log, OSM_LOG_VERBOSE,
-			"Received optimized SLtoVL get response in_port_num %u "
-			"out_port_num %u with GUID 0x%" PRIx64 " for parent "
-			"node GUID 0x%" PRIx64 ", TID 0x%" PRIx64 "\n",
-			in_port_num, out_port_num, cl_ntoh64(port_guid),
-			cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
-
-	osm_dump_slvl_map_table(sm->p_log, port_guid, in_port_num,
-				out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
-
-	if (attr_mod & 0x10000) {
-		startoutport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
-		endoutport = osm_node_get_num_physp(p_node);
-	} else
-		endoutport = startoutport = out_port_num;
-	if (attr_mod & 0x20000) {
-		startinport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
-		endinport = osm_node_get_num_physp(p_node);
-	} else
-		endinport = startinport = in_port_num;
-
-	for (out_port_num = startoutport; out_port_num < endoutport;
-	     out_port_num++) {
-		p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
-		for (in_port_num = startinport; in_port_num < endinport;
-		     in_port_num++)
-			osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
+	OSM_LOG(sm->p_log, OSM_LOG_DEBUG, "Received SLtoVL GetResp"
+		" in_port_num %u out_port_num %u with GUID 0x%" PRIx64
+		" for parent node GUID 0x%" PRIx64 ", TID 0x%" PRIx64 "\n",
+		startinport == endinport ? startinport : 0xff,
+		startoutport == endoutport ? startoutport : 0xff,
+		cl_ntoh64(port_guid), cl_ntoh64(node_guid),
+		cl_ntoh64(p_smp->trans_id));
+
+	osm_dump_slvl_map_table(sm->p_log, port_guid,
+				startinport == endinport ? startinport : 0xff,
+				startoutport == endoutport ? startoutport : 0xff,
+				p_slvl_tbl, OSM_LOG_DEBUG);
+
+	for (out_port = startoutport; out_port <= endoutport; out_port++) {
+		p_physp = osm_node_get_physp_ptr(p_node, out_port);
+		for (in_port = startinport; in_port <= endinport; in_port++)
+			osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port);
 	}
 
 Exit:


> Removed unneeded check of p_physp in input port loop in osm_slvl_rcv_process
> for optimized sl2vl mapping programming
> 
> Changes since v1:
> Changed optimized programming flow in osm_qos to only call
> sl2vl_update once per switch supporting this feature
> 
> diff --git a/opensm/include/opensm/osm_subnet.h b/opensm/include/opensm/osm_subnet.h
> index 3c08689..583c070 100644
> --- a/opensm/include/opensm/osm_subnet.h
> +++ b/opensm/include/opensm/osm_subnet.h
> @@ -206,6 +206,7 @@ typedef struct osm_subn_opt {
>  	boolean_t daemon;
>  	boolean_t sm_inactive;
>  	boolean_t babbling_port_policy;
> +	boolean_t use_optimized_slvl;
>  	osm_qos_options_t qos_options;
>  	osm_qos_options_t qos_ca_options;
>  	osm_qos_options_t qos_sw0_options;
> @@ -433,6 +434,10 @@ typedef struct osm_subn_opt {
>  *	babbling_port_policy
>  *		OpenSM will enforce its "babbling" port policy.
>  *
> +*	use_optimized_slvl
> +*		Use optimized SLtoVLMappingTable programming if
> +*		device indicates it supports this.
> +*
>  *	perfmgr
>  *		Enable or disable the performance manager
>  *
> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> index 08f9a60..617a86e 100644
> --- a/opensm/opensm/osm_qos.c
> +++ b/opensm/opensm/osm_qos.c

I'm yet reviewing the changes in osm_qos.c.

> @@ -1,5 +1,6 @@
>  /*
>   * Copyright (c) 2006-2009 Voltaire, Inc. All rights reserved.
> + * Copyright (c) 2009 HNR Consulting. All rights reserved.
>   *
>   * This software is available to you under a choice of one of two
>   * licenses.  You may choose to be licensed under the terms of the GNU
> @@ -150,7 +151,7 @@ static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
>  
>  static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>  					  uint8_t in_port, uint8_t out_port,
> -					  unsigned force_update,
> +					  unsigned optimize, unsigned force_update,
>  					  const ib_slvl_table_t * sl2vl_table)
>  {
>  	osm_madw_context_t context;
> @@ -180,41 +181,47 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>  	context.slvl_context.node_guid = osm_node_get_node_guid(p_node);
>  	context.slvl_context.port_guid = osm_physp_get_port_guid(p);
>  	context.slvl_context.set_method = TRUE;
> -	attr_mod = in_port << 8 | out_port;
> +	if (optimize)
> +		/* wildcard both input and output ports */
> +		attr_mod = 0x30000;
> +	else
> +		attr_mod = in_port << 8 | out_port;
>  	return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
>  			   (uint8_t *) & tbl, sizeof(tbl),
>  			   IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
>  			   CL_DISP_MSGID_NONE, &context);
>  }
>  
> -static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
> +static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_physp_t * p0,
>  				    osm_physp_t * p, uint8_t port_num,
> -				    unsigned force_update,
> +				    unsigned optimize, unsigned force_update,
>  				    const struct qos_config *qcfg)
>  {
>  	ib_api_status_t status;
>  	uint8_t i, num_ports;
> -	osm_physp_t *p_physp;
> +	osm_node_t *p_node;
>  
> -	if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH) {
> +	p_node = osm_physp_get_node_ptr(p);
> +	if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
>  		if (ib_port_info_get_vl_cap(&p->port_info) == 1) {
>  			/* Check port 0's capability mask */
> -			p_physp = p_port->p_physp;
> -			if (!
> -			    (p_physp->port_info.
> -			     capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> +			if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>  				return IB_SUCCESS;
>  		}
> -		num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
> +		num_ports = osm_node_get_num_physp(p_node);
>  	} else {
>  		if (!(p->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>  			return IB_SUCCESS;
>  		num_ports = 1;
>  	}
>  
> +	if (optimize)
> +		return sl2vl_update_table(sm, p, 1, port_num, optimize,
> +					  force_update, &qcfg->sl2vl);
> +
>  	for (i = 0; i < num_ports; i++) {
> -		status = sl2vl_update_table(sm, p, i, port_num, force_update,
> -					    &qcfg->sl2vl);
> +		status = sl2vl_update_table(sm, p, i, port_num, optimize,
> +					    force_update, &qcfg->sl2vl);
>  		if (status != IB_SUCCESS)
>  			return status;
>  	}
> @@ -222,22 +229,30 @@ static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
>  	return IB_SUCCESS;
>  }
>  
> -static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
> -			   osm_port_t * p_port, osm_physp_t * p,
> -			   uint8_t port_num, unsigned force_update,
> -			   const struct qos_config *qcfg)
> +static int vlarb_physp_setup(osm_sm_t * sm, osm_physp_t * p, uint8_t port_num,
> +			     unsigned force_update,
> +			     const struct qos_config *qcfg)
>  {
> -	ib_api_status_t status;
> -
>  	/* OpVLs should be ok at this moment - just use it */
>  
>  	/* setup VL high limit on the physp later to be updated by link mgr */
>  	p->vl_high_limit = qcfg->vl_high_limit;
>  
>  	/* setup VLArbitration */
> -	status = vlarb_update(sm, p, port_num, force_update, qcfg);
> +	return vlarb_update(sm, p, port_num, force_update, qcfg);
> +}
> +
> +static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
> +			   osm_physp_t * p0, osm_physp_t * p,
> +			   uint8_t port_num, unsigned force_update,
> +			   const struct qos_config *qcfg)
> +{
> +	ib_api_status_t status;
> +
> +	/* setup VLArbitration */
> +	status = vlarb_physp_setup(sm, p, port_num, force_update, qcfg);
>  	if (status != IB_SUCCESS) {
> -		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6202 : "
> +		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6202: "
>  			"failed to update VLArbitration tables "
>  			"for port %" PRIx64 " #%d\n",
>  			cl_ntoh64(p->port_guid), port_num);
> @@ -245,9 +260,9 @@ static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
>  	}
>  
>  	/* setup SL2VL tables */
> -	status = sl2vl_update(sm, p_port, p, port_num, force_update, qcfg);
> +	status = sl2vl_update(sm, p0, p, port_num, 0, force_update, qcfg);
>  	if (status != IB_SUCCESS) {
> -		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6203 : "
> +		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6203: "
>  			"failed to update SL2VLMapping tables "
>  			"for port %" PRIx64 " #%d\n",
>  			cl_ntoh64(p->port_guid), port_num);
> @@ -267,6 +282,7 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>  	uint32_t num_physp;
>  	osm_physp_t *p_physp;
>  	osm_node_t *p_node;
> +	osm_switch_t *p_sw;
>  	unsigned force_update;
>  	int ret = 0;
>  	uint8_t i;
> @@ -290,6 +306,44 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>  	/* read QoS policy config file */
>  	osm_qos_parse_policy_file(&p_osm->subn);
>  
> +	/* loop on switches that support optimized SL2VL programming first */
> +	p_tbl = &p_osm->subn.sw_guid_tbl;
> +	p_next = cl_qmap_head(p_tbl);
> +	while (p_next != cl_qmap_end(p_tbl)) {
> +		p_sw = (osm_switch_t *) p_next;
> +		p_next = cl_qmap_next(p_next);
> +
> +		if (ib_switch_info_get_opt_sl2vlmapping(&p_sw->switch_info) &&
> +		    p_osm->subn.opt.use_optimized_slvl) {
> +			p_physp = osm_node_get_physp_ptr(p_sw->p_node, 1);
> +			num_physp = osm_node_get_num_physp(p_sw->p_node);
> +			force_update = p_osm->subn.need_update;
> +			for (i = 1; i < num_physp; i++) {
> +				p_physp = osm_node_get_physp_ptr(p_sw->p_node, i);
> +				if (!p_physp)
> +					continue;
> +				if (vlarb_physp_setup(&p_osm->sm, p_physp, i,
> +						      p_physp->need_update ||
> +						      p_osm->subn.need_update,
> +						      &swe_config))
> +					ret = -1;
> +				force_update |= p_physp->need_update;
> +			}
> +			if (sl2vl_update(&p_osm->sm,
> +					 osm_node_get_physp_ptr(p_sw->p_node, 0),
> +					 p_physp, i, 1, force_update,
> +					 &swe_config)) {
> +				OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 6204: "
> +					"failed to update optimized SL2VLMapping"
> +					" tables for port %" PRIx64 " #%d\n",
> +					cl_ntoh64(p_physp->port_guid), i);
> +				ret = -1;
> +			}
> +		}
> +	}
> +
> +	/* now, loop on ports skipping the external ports of switches
> +	   that support optimized SL2VL programming */
>  	p_tbl = &p_osm->subn.port_guid_tbl;
>  	p_next = cl_qmap_head(p_tbl);
>  	while (p_next != cl_qmap_end(p_tbl)) {
> @@ -298,6 +352,11 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>  
>  		p_node = p_port->p_node;
>  		if (p_node->sw) {
> +			/* skip switches with optimized SL2VL mapping
> +			   programming since already done */
> +			if (ib_switch_info_get_opt_sl2vlmapping(&p_node->sw->switch_info) &&
> +			    p_osm->subn.opt.use_optimized_slvl)
> +				goto check_port0;
>  			num_physp = osm_node_get_num_physp(p_node);
>  			for (i = 1; i < num_physp; i++) {
>  				p_physp = osm_node_get_physp_ptr(p_node, i);
> @@ -306,10 +365,11 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>  				force_update = p_physp->need_update ||
>  				    p_osm->subn.need_update;
>  				if (qos_physp_setup(&p_osm->log, &p_osm->sm,
> -						    p_port, p_physp, i,
> +						    p_port->p_physp, p_physp, i,
>  						    force_update, &swe_config))
>  					ret = -1;
>  			}
> +check_port0:
>  			/* skip base port 0 */
>  			if (!ib_switch_info_is_enhanced_port0
>  			    (&p_node->sw->switch_info))
> @@ -326,8 +386,8 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>  			continue;
>  
>  		force_update = p_physp->need_update || p_osm->subn.need_update;
> -		if (qos_physp_setup(&p_osm->log, &p_osm->sm, p_port, p_physp,
> -				    0, force_update, cfg))
> +		if (qos_physp_setup(&p_osm->log, &p_osm->sm, p_port->p_physp,
> +				    p_physp, 0, force_update, cfg))
>  			ret = -1;
>  	}
>  
> diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
> index 4f75690..214a763 100644
> --- a/opensm/opensm/osm_slvl_map_rcv.c
> +++ b/opensm/opensm/osm_slvl_map_rcv.c
> @@ -2,6 +2,7 @@
>   * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
>   * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
>   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
> + * Copyright (c) 2009 HNR Consulting. All rights reserved.
>   *
>   * This software is available to you under a choice of one of two
>   * licenses.  You may choose to be licensed under the terms of the GNU
> @@ -70,7 +71,9 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>  	osm_slvl_context_t *p_context;
>  	ib_net64_t port_guid;
>  	ib_net64_t node_guid;
> -	uint8_t out_port_num, in_port_num;
> +	uint32_t attr_mod;
> +	uint8_t out_port_num, in_port_num, startinport, startoutport,
> +		endinport, endoutport;
>  
>  	CL_ASSERT(sm);
>  
> @@ -109,6 +112,9 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>  		    (uint8_t) cl_ntoh32(p_smp->attr_mod & 0xFF000000);
>  		in_port_num =
>  		    (uint8_t) cl_ntoh32((p_smp->attr_mod & 0x00FF0000) << 8);
> +		attr_mod = cl_ntoh32(p_smp->attr_mod);
> +		if (attr_mod & 0x30000)
> +			goto opt_sl2vl;
>  		p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
>  	} else {
>  		p_physp = p_port->p_physp;
> @@ -121,7 +127,7 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>  	   all we want is to update the subnet.
>  	 */
>  	OSM_LOG(sm->p_log, OSM_LOG_VERBOSE,
> -		"Got SLtoVL get response in_port_num %u out_port_num %u with "
> +		"Received SLtoVL GetResp in_port_num %u out_port_num %u with "
>  		"GUID 0x%" PRIx64 " for parent node GUID 0x%" PRIx64 ", TID 0x%"
>  		PRIx64 "\n", in_port_num, out_port_num, cl_ntoh64(port_guid),
>  		cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
> @@ -140,6 +146,38 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>  				out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
>  
>  	osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
> +	goto Exit;
> +
> +opt_sl2vl:
> +	if (osm_log_is_active(sm->p_log, OSM_LOG_VERBOSE))
> +		osm_log(sm->p_log, OSM_LOG_VERBOSE,
> +			"Received optimized SLtoVL get response in_port_num %u "
> +			"out_port_num %u with GUID 0x%" PRIx64 " for parent "
> +			"node GUID 0x%" PRIx64 ", TID 0x%" PRIx64 "\n",
> +			in_port_num, out_port_num, cl_ntoh64(port_guid),
> +			cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
> +
> +	osm_dump_slvl_map_table(sm->p_log, port_guid, in_port_num,
> +				out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
> +
> +	if (attr_mod & 0x10000) {
> +		startoutport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
> +		endoutport = osm_node_get_num_physp(p_node);
> +	} else
> +		endoutport = startoutport = out_port_num;

For this case (when endoutport == startoutport) in later loop:

	for (out_port_num = startoutport; out_port_num < endoutport;
	     out_port_num++)

nothing will be done. Right?

> +	if (attr_mod & 0x20000) {
> +		startinport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
> +		endinport = osm_node_get_num_physp(p_node);
> +	} else
> +		endinport = startinport = in_port_num;

Ditto with 'inport'.

Sasha

> +
> +	for (out_port_num = startoutport; out_port_num < endoutport;
> +	     out_port_num++) {
> +		p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
> +		for (in_port_num = startinport; in_port_num < endinport;
> +		     in_port_num++)
> +			osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
> +	}
>  
>  Exit:
>  	cl_plock_release(sm->p_lock);
> diff --git a/opensm/opensm/osm_subnet.c b/opensm/opensm/osm_subnet.c
> index 09029db..103b5c6 100644
> --- a/opensm/opensm/osm_subnet.c
> +++ b/opensm/opensm/osm_subnet.c
> @@ -354,6 +354,7 @@ static const opt_rec_t opt_tbl[] = {
>  	{ "daemon", OPT_OFFSET(daemon), opts_parse_boolean, NULL, 0 },
>  	{ "sm_inactive", OPT_OFFSET(sm_inactive), opts_parse_boolean, NULL, 1 },
>  	{ "babbling_port_policy", OPT_OFFSET(babbling_port_policy), opts_parse_boolean, NULL, 1 },
> +	{ "use_optimized_slvl", OPT_OFFSET(use_optimized_slvl), opts_parse_boolean, NULL, 1 },
>  #ifdef ENABLE_OSM_PERF_MGR
>  	{ "perfmgr", OPT_OFFSET(perfmgr), opts_parse_boolean, NULL, 0 },
>  	{ "perfmgr_redir", OPT_OFFSET(perfmgr_redir), opts_parse_boolean, NULL, 0 },
> @@ -713,6 +714,7 @@ void osm_subn_set_default_opt(IN osm_subn_opt_t * p_opt)
>  	p_opt->daemon = FALSE;
>  	p_opt->sm_inactive = FALSE;
>  	p_opt->babbling_port_policy = FALSE;
> +	p_opt->use_optimized_slvl = FALSE;
>  #ifdef ENABLE_OSM_PERF_MGR
>  	p_opt->perfmgr = FALSE;
>  	p_opt->perfmgr_redir = TRUE;
> @@ -1499,10 +1501,13 @@ int osm_subn_output_conf(FILE *out, IN osm_subn_opt_t * p_opts)
>  		"# SM Inactive\n"
>  		"sm_inactive %s\n\n"
>  		"# Babbling Port Policy\n"
> -		"babbling_port_policy %s\n\n",
> +		"babbling_port_policy %s\n\n"
> +		"# Use Optimized SLtoVLMapping programming if supported by device\n"
> +		"use_optimized_slvl %s\n\n",
>  		p_opts->daemon ? "TRUE" : "FALSE",
>  		p_opts->sm_inactive ? "TRUE" : "FALSE",
> -		p_opts->babbling_port_policy ? "TRUE" : "FALSE");
> +		p_opts->babbling_port_policy ? "TRUE" : "FALSE",
> +		p_opts->use_optimized_slvl ? "TRUE" : "FALSE");
>  
>  #ifdef ENABLE_OSM_PERF_MGR
>  	fprintf(out,
> --
> 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
> 
--
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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
  2009-12-29 16:29   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
@ 2009-12-30 16:08     ` Hal Rosenstock
  0 siblings, 0 replies; 37+ messages in thread
From: Hal Rosenstock @ 2009-12-30 16:08 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Hi Sasha,

On Tue, Dec 29, 2009 at 11:29 AM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
> Hi Hal,
>
> On 14:41 Tue 01 Dec     , Hal Rosenstock wrote:
>>
>> Optimized SLtoVLMappingTable programming reduces the number of MADs
>> needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
>> SLtoVLMappingTable.
>>
>> Signed-off-by: Hal Rosenstock <hal.rosenstock@gmail.com>
>> ---
>> Changes since v2:
>> Use osm_log rather than OSM_LOG in osm_slvl_rcv_process for optimized
>> sl2vl mapping programming verbose log message
>
> I didn't ask for such change. Instead I asked to avoid the function
> (osm_slvl_rcv_process() in this patch) constructed in form of:
>
>        if (cond1)
>                goto flow1;
>
>        do_flow2;
>
>        goto end;
>
> flow1:
>        do_flow1;
>
> end:
>
> In general this is equivalent to simpler and more readable:
>
>        if (cond1)
>                do_flow1();
>        else
>                do_flow2();
>
> And in case of osm_slvl_rcv_process() even nicer merge is possible when
> slvl port table setup loop is shared, but its range (start*port and
> end*port values) is defined differently for various input cases. Just
> like this (against your patch, rebased over recent fixes, compilation
> only tested):
>
>
> diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
> index 7ca0b8f..73c6253 100644
> --- a/opensm/opensm/osm_slvl_map_rcv.c
> +++ b/opensm/opensm/osm_slvl_map_rcv.c
> @@ -72,8 +72,8 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>        ib_net64_t port_guid;
>        ib_net64_t node_guid;
>        uint32_t attr_mod;
> -       uint8_t out_port_num, in_port_num, startinport, startoutport,
> -               endinport, endoutport;
> +       uint8_t startinport, endinport, startoutport, endoutport;
> +       uint8_t in_port, out_port;
>
>        CL_ASSERT(sm);
>
> @@ -109,80 +109,51 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>        if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
>                unsigned num_ports = osm_node_get_num_physp(p_node) - 1;
>                attr_mod = cl_ntoh32(p_smp->attr_mod);
> -               out_port_num = attr_mod & 0xff;
> -               in_port_num = (attr_mod >> 8) & 0xff;
> -               if (in_port_num > num_ports || out_port_num > num_ports) {
> +
> +               if (attr_mod & 0x10000) {
> +                       startoutport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
> +                       endoutport = osm_node_get_num_physp(p_node) - 1;
> +               } else
> +                       startoutport = endoutport = attr_mod & 0xff;
> +
> +               if (attr_mod & 0x20000) {
> +                       startinport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
> +                       endinport = osm_node_get_num_physp(p_node) - 1;
> +               } else
> +                       startinport = endinport = (attr_mod >> 8) & 0xff;
> +
> +               if (startinport > num_ports || startoutport > num_ports) {
>                        OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 2C07"
>                                "Invalid attribute modifier 0x%x reveived in"
>                                " response from switch 0x%" PRIx64 "\n",
> -                               cl_ntoh32(p_smp->attr_mod),
> -                               cl_ntoh64(node_guid));
> +                               cl_ntoh32(attr_mod), cl_ntoh64(node_guid));
>                        goto Exit;
>                }
> -               if (attr_mod & 0x30000)
> -                       goto opt_sl2vl;
> -               p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
> -       } else {
> -               p_physp = p_port->p_physp;
> -               out_port_num = p_physp->port_num;
> -               in_port_num = 0;
> -       }
>
> -       /*
> -          We do not care if this is a result of a set or get -
> -          all we want is to update the subnet.
> -        */
> -       OSM_LOG(sm->p_log, OSM_LOG_VERBOSE,
> -               "Received SLtoVL GetResp in_port_num %u out_port_num %u with "
> -               "GUID 0x%" PRIx64 " for parent node GUID 0x%" PRIx64 ", TID 0x%"
> -               PRIx64 "\n", in_port_num, out_port_num, cl_ntoh64(port_guid),
> -               cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
> -
> -       /*
> -          Determine if we encountered a new Physical Port.
> -          If so, Ignore it.
> -        */
> -       if (!p_physp) {
> -               OSM_LOG(sm->p_log, OSM_LOG_ERROR,
> -                       "Got invalid port number %u\n", out_port_num);
> -               goto Exit;
> +       } else {
> +               startoutport = endoutport = p_port->p_physp->port_num;
> +               startinport = endinport = 0;
>        }
>
> -       osm_dump_slvl_map_table(sm->p_log, port_guid, in_port_num,
> -                               out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
> -
> -       osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
> -       goto Exit;
> -
> -opt_sl2vl:
> -       if (osm_log_is_active(sm->p_log, OSM_LOG_VERBOSE))
> -               osm_log(sm->p_log, OSM_LOG_VERBOSE,
> -                       "Received optimized SLtoVL get response in_port_num %u "
> -                       "out_port_num %u with GUID 0x%" PRIx64 " for parent "
> -                       "node GUID 0x%" PRIx64 ", TID 0x%" PRIx64 "\n",
> -                       in_port_num, out_port_num, cl_ntoh64(port_guid),
> -                       cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
> -
> -       osm_dump_slvl_map_table(sm->p_log, port_guid, in_port_num,
> -                               out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
> -
> -       if (attr_mod & 0x10000) {
> -               startoutport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
> -               endoutport = osm_node_get_num_physp(p_node);
> -       } else
> -               endoutport = startoutport = out_port_num;
> -       if (attr_mod & 0x20000) {
> -               startinport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
> -               endinport = osm_node_get_num_physp(p_node);
> -       } else
> -               endinport = startinport = in_port_num;
> -
> -       for (out_port_num = startoutport; out_port_num < endoutport;
> -            out_port_num++) {
> -               p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
> -               for (in_port_num = startinport; in_port_num < endinport;
> -                    in_port_num++)
> -                       osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
> +       OSM_LOG(sm->p_log, OSM_LOG_DEBUG, "Received SLtoVL GetResp"
> +               " in_port_num %u out_port_num %u with GUID 0x%" PRIx64
> +               " for parent node GUID 0x%" PRIx64 ", TID 0x%" PRIx64 "\n",
> +               startinport == endinport ? startinport : 0xff,
> +               startoutport == endoutport ? startoutport : 0xff,
> +               cl_ntoh64(port_guid), cl_ntoh64(node_guid),
> +               cl_ntoh64(p_smp->trans_id));
> +
> +       osm_dump_slvl_map_table(sm->p_log, port_guid,
> +                               startinport == endinport ? startinport : 0xff,
> +                               startoutport == endoutport ? startoutport : 0xff,
> +                               p_slvl_tbl, OSM_LOG_DEBUG);
> +
> +       for (out_port = startoutport; out_port <= endoutport; out_port++) {
> +               p_physp = osm_node_get_physp_ptr(p_node, out_port);
> +               for (in_port = startinport; in_port <= endinport; in_port++)
> +                       osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port);
>        }
>
>  Exit:

This patch looks good to me and appears to be working in both the non
optimized and optimized cases although there is at least one subtle
change and check removal which should be OK (not sure what the
original motivation was).

>> Removed unneeded check of p_physp in input port loop in osm_slvl_rcv_process
>> for optimized sl2vl mapping programming
>>
>> Changes since v1:
>> Changed optimized programming flow in osm_qos to only call
>> sl2vl_update once per switch supporting this feature
>>
>> diff --git a/opensm/include/opensm/osm_subnet.h b/opensm/include/opensm/osm_subnet.h
>> index 3c08689..583c070 100644
>> --- a/opensm/include/opensm/osm_subnet.h
>> +++ b/opensm/include/opensm/osm_subnet.h
>> @@ -206,6 +206,7 @@ typedef struct osm_subn_opt {
>>       boolean_t daemon;
>>       boolean_t sm_inactive;
>>       boolean_t babbling_port_policy;
>> +     boolean_t use_optimized_slvl;
>>       osm_qos_options_t qos_options;
>>       osm_qos_options_t qos_ca_options;
>>       osm_qos_options_t qos_sw0_options;
>> @@ -433,6 +434,10 @@ typedef struct osm_subn_opt {
>>  *    babbling_port_policy
>>  *            OpenSM will enforce its "babbling" port policy.
>>  *
>> +*    use_optimized_slvl
>> +*            Use optimized SLtoVLMappingTable programming if
>> +*            device indicates it supports this.
>> +*
>>  *    perfmgr
>>  *            Enable or disable the performance manager
>>  *
>> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
>> index 08f9a60..617a86e 100644
>> --- a/opensm/opensm/osm_qos.c
>> +++ b/opensm/opensm/osm_qos.c
>
> I'm yet reviewing the changes in osm_qos.c.
>
>> @@ -1,5 +1,6 @@
>>  /*
>>   * Copyright (c) 2006-2009 Voltaire, Inc. All rights reserved.
>> + * Copyright (c) 2009 HNR Consulting. All rights reserved.
>>   *
>>   * This software is available to you under a choice of one of two
>>   * licenses.  You may choose to be licensed under the terms of the GNU
>> @@ -150,7 +151,7 @@ static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
>>
>>  static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>>                                         uint8_t in_port, uint8_t out_port,
>> -                                       unsigned force_update,
>> +                                       unsigned optimize, unsigned force_update,
>>                                         const ib_slvl_table_t * sl2vl_table)
>>  {
>>       osm_madw_context_t context;
>> @@ -180,41 +181,47 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>>       context.slvl_context.node_guid = osm_node_get_node_guid(p_node);
>>       context.slvl_context.port_guid = osm_physp_get_port_guid(p);
>>       context.slvl_context.set_method = TRUE;
>> -     attr_mod = in_port << 8 | out_port;
>> +     if (optimize)
>> +             /* wildcard both input and output ports */
>> +             attr_mod = 0x30000;
>> +     else
>> +             attr_mod = in_port << 8 | out_port;
>>       return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
>>                          (uint8_t *) & tbl, sizeof(tbl),
>>                          IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
>>                          CL_DISP_MSGID_NONE, &context);
>>  }
>>
>> -static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
>> +static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_physp_t * p0,
>>                                   osm_physp_t * p, uint8_t port_num,
>> -                                 unsigned force_update,
>> +                                 unsigned optimize, unsigned force_update,
>>                                   const struct qos_config *qcfg)
>>  {
>>       ib_api_status_t status;
>>       uint8_t i, num_ports;
>> -     osm_physp_t *p_physp;
>> +     osm_node_t *p_node;
>>
>> -     if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH) {
>> +     p_node = osm_physp_get_node_ptr(p);
>> +     if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
>>               if (ib_port_info_get_vl_cap(&p->port_info) == 1) {
>>                       /* Check port 0's capability mask */
>> -                     p_physp = p_port->p_physp;
>> -                     if (!
>> -                         (p_physp->port_info.
>> -                          capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>> +                     if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>>                               return IB_SUCCESS;
>>               }
>> -             num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
>> +             num_ports = osm_node_get_num_physp(p_node);
>>       } else {
>>               if (!(p->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>>                       return IB_SUCCESS;
>>               num_ports = 1;
>>       }
>>
>> +     if (optimize)
>> +             return sl2vl_update_table(sm, p, 1, port_num, optimize,
>> +                                       force_update, &qcfg->sl2vl);
>> +
>>       for (i = 0; i < num_ports; i++) {
>> -             status = sl2vl_update_table(sm, p, i, port_num, force_update,
>> -                                         &qcfg->sl2vl);
>> +             status = sl2vl_update_table(sm, p, i, port_num, optimize,
>> +                                         force_update, &qcfg->sl2vl);
>>               if (status != IB_SUCCESS)
>>                       return status;
>>       }
>> @@ -222,22 +229,30 @@ static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
>>       return IB_SUCCESS;
>>  }
>>
>> -static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
>> -                        osm_port_t * p_port, osm_physp_t * p,
>> -                        uint8_t port_num, unsigned force_update,
>> -                        const struct qos_config *qcfg)
>> +static int vlarb_physp_setup(osm_sm_t * sm, osm_physp_t * p, uint8_t port_num,
>> +                          unsigned force_update,
>> +                          const struct qos_config *qcfg)
>>  {
>> -     ib_api_status_t status;
>> -
>>       /* OpVLs should be ok at this moment - just use it */
>>
>>       /* setup VL high limit on the physp later to be updated by link mgr */
>>       p->vl_high_limit = qcfg->vl_high_limit;
>>
>>       /* setup VLArbitration */
>> -     status = vlarb_update(sm, p, port_num, force_update, qcfg);
>> +     return vlarb_update(sm, p, port_num, force_update, qcfg);
>> +}
>> +
>> +static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
>> +                        osm_physp_t * p0, osm_physp_t * p,
>> +                        uint8_t port_num, unsigned force_update,
>> +                        const struct qos_config *qcfg)
>> +{
>> +     ib_api_status_t status;
>> +
>> +     /* setup VLArbitration */
>> +     status = vlarb_physp_setup(sm, p, port_num, force_update, qcfg);
>>       if (status != IB_SUCCESS) {
>> -             OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6202 : "
>> +             OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6202: "
>>                       "failed to update VLArbitration tables "
>>                       "for port %" PRIx64 " #%d\n",
>>                       cl_ntoh64(p->port_guid), port_num);
>> @@ -245,9 +260,9 @@ static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
>>       }
>>
>>       /* setup SL2VL tables */
>> -     status = sl2vl_update(sm, p_port, p, port_num, force_update, qcfg);
>> +     status = sl2vl_update(sm, p0, p, port_num, 0, force_update, qcfg);
>>       if (status != IB_SUCCESS) {
>> -             OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6203 : "
>> +             OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6203: "
>>                       "failed to update SL2VLMapping tables "
>>                       "for port %" PRIx64 " #%d\n",
>>                       cl_ntoh64(p->port_guid), port_num);
>> @@ -267,6 +282,7 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>>       uint32_t num_physp;
>>       osm_physp_t *p_physp;
>>       osm_node_t *p_node;
>> +     osm_switch_t *p_sw;
>>       unsigned force_update;
>>       int ret = 0;
>>       uint8_t i;
>> @@ -290,6 +306,44 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>>       /* read QoS policy config file */
>>       osm_qos_parse_policy_file(&p_osm->subn);
>>
>> +     /* loop on switches that support optimized SL2VL programming first */
>> +     p_tbl = &p_osm->subn.sw_guid_tbl;
>> +     p_next = cl_qmap_head(p_tbl);
>> +     while (p_next != cl_qmap_end(p_tbl)) {
>> +             p_sw = (osm_switch_t *) p_next;
>> +             p_next = cl_qmap_next(p_next);
>> +
>> +             if (ib_switch_info_get_opt_sl2vlmapping(&p_sw->switch_info) &&
>> +                 p_osm->subn.opt.use_optimized_slvl) {
>> +                     p_physp = osm_node_get_physp_ptr(p_sw->p_node, 1);
>> +                     num_physp = osm_node_get_num_physp(p_sw->p_node);
>> +                     force_update = p_osm->subn.need_update;
>> +                     for (i = 1; i < num_physp; i++) {
>> +                             p_physp = osm_node_get_physp_ptr(p_sw->p_node, i);
>> +                             if (!p_physp)
>> +                                     continue;
>> +                             if (vlarb_physp_setup(&p_osm->sm, p_physp, i,
>> +                                                   p_physp->need_update ||
>> +                                                   p_osm->subn.need_update,
>> +                                                   &swe_config))
>> +                                     ret = -1;
>> +                             force_update |= p_physp->need_update;
>> +                     }
>> +                     if (sl2vl_update(&p_osm->sm,
>> +                                      osm_node_get_physp_ptr(p_sw->p_node, 0),
>> +                                      p_physp, i, 1, force_update,
>> +                                      &swe_config)) {
>> +                             OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 6204: "
>> +                                     "failed to update optimized SL2VLMapping"
>> +                                     " tables for port %" PRIx64 " #%d\n",
>> +                                     cl_ntoh64(p_physp->port_guid), i);
>> +                             ret = -1;
>> +                     }
>> +             }
>> +     }
>> +
>> +     /* now, loop on ports skipping the external ports of switches
>> +        that support optimized SL2VL programming */
>>       p_tbl = &p_osm->subn.port_guid_tbl;
>>       p_next = cl_qmap_head(p_tbl);
>>       while (p_next != cl_qmap_end(p_tbl)) {
>> @@ -298,6 +352,11 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>>
>>               p_node = p_port->p_node;
>>               if (p_node->sw) {
>> +                     /* skip switches with optimized SL2VL mapping
>> +                        programming since already done */
>> +                     if (ib_switch_info_get_opt_sl2vlmapping(&p_node->sw->switch_info) &&
>> +                         p_osm->subn.opt.use_optimized_slvl)
>> +                             goto check_port0;
>>                       num_physp = osm_node_get_num_physp(p_node);
>>                       for (i = 1; i < num_physp; i++) {
>>                               p_physp = osm_node_get_physp_ptr(p_node, i);
>> @@ -306,10 +365,11 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>>                               force_update = p_physp->need_update ||
>>                                   p_osm->subn.need_update;
>>                               if (qos_physp_setup(&p_osm->log, &p_osm->sm,
>> -                                                 p_port, p_physp, i,
>> +                                                 p_port->p_physp, p_physp, i,
>>                                                   force_update, &swe_config))
>>                                       ret = -1;
>>                       }
>> +check_port0:
>>                       /* skip base port 0 */
>>                       if (!ib_switch_info_is_enhanced_port0
>>                           (&p_node->sw->switch_info))
>> @@ -326,8 +386,8 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>>                       continue;
>>
>>               force_update = p_physp->need_update || p_osm->subn.need_update;
>> -             if (qos_physp_setup(&p_osm->log, &p_osm->sm, p_port, p_physp,
>> -                                 0, force_update, cfg))
>> +             if (qos_physp_setup(&p_osm->log, &p_osm->sm, p_port->p_physp,
>> +                                 p_physp, 0, force_update, cfg))
>>                       ret = -1;
>>       }
>>
>> diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
>> index 4f75690..214a763 100644
>> --- a/opensm/opensm/osm_slvl_map_rcv.c
>> +++ b/opensm/opensm/osm_slvl_map_rcv.c
>> @@ -2,6 +2,7 @@
>>   * Copyright (c) 2004-2009 Voltaire, Inc. All rights reserved.
>>   * Copyright (c) 2002-2005 Mellanox Technologies LTD. All rights reserved.
>>   * Copyright (c) 1996-2003 Intel Corporation. All rights reserved.
>> + * Copyright (c) 2009 HNR Consulting. All rights reserved.
>>   *
>>   * This software is available to you under a choice of one of two
>>   * licenses.  You may choose to be licensed under the terms of the GNU
>> @@ -70,7 +71,9 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>>       osm_slvl_context_t *p_context;
>>       ib_net64_t port_guid;
>>       ib_net64_t node_guid;
>> -     uint8_t out_port_num, in_port_num;
>> +     uint32_t attr_mod;
>> +     uint8_t out_port_num, in_port_num, startinport, startoutport,
>> +             endinport, endoutport;
>>
>>       CL_ASSERT(sm);
>>
>> @@ -109,6 +112,9 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>>                   (uint8_t) cl_ntoh32(p_smp->attr_mod & 0xFF000000);
>>               in_port_num =
>>                   (uint8_t) cl_ntoh32((p_smp->attr_mod & 0x00FF0000) << 8);
>> +             attr_mod = cl_ntoh32(p_smp->attr_mod);
>> +             if (attr_mod & 0x30000)
>> +                     goto opt_sl2vl;
>>               p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
>>       } else {
>>               p_physp = p_port->p_physp;
>> @@ -121,7 +127,7 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>>          all we want is to update the subnet.
>>        */
>>       OSM_LOG(sm->p_log, OSM_LOG_VERBOSE,
>> -             "Got SLtoVL get response in_port_num %u out_port_num %u with "
>> +             "Received SLtoVL GetResp in_port_num %u out_port_num %u with "
>>               "GUID 0x%" PRIx64 " for parent node GUID 0x%" PRIx64 ", TID 0x%"
>>               PRIx64 "\n", in_port_num, out_port_num, cl_ntoh64(port_guid),
>>               cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
>> @@ -140,6 +146,38 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>>                               out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
>>
>>       osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
>> +     goto Exit;
>> +
>> +opt_sl2vl:
>> +     if (osm_log_is_active(sm->p_log, OSM_LOG_VERBOSE))
>> +             osm_log(sm->p_log, OSM_LOG_VERBOSE,
>> +                     "Received optimized SLtoVL get response in_port_num %u "
>> +                     "out_port_num %u with GUID 0x%" PRIx64 " for parent "
>> +                     "node GUID 0x%" PRIx64 ", TID 0x%" PRIx64 "\n",
>> +                     in_port_num, out_port_num, cl_ntoh64(port_guid),
>> +                     cl_ntoh64(node_guid), cl_ntoh64(p_smp->trans_id));
>> +
>> +     osm_dump_slvl_map_table(sm->p_log, port_guid, in_port_num,
>> +                             out_port_num, p_slvl_tbl, OSM_LOG_DEBUG);
>> +
>> +     if (attr_mod & 0x10000) {
>> +             startoutport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
>> +             endoutport = osm_node_get_num_physp(p_node);
>> +     } else
>> +             endoutport = startoutport = out_port_num;
>
> For this case (when endoutport == startoutport) in later loop:
>
>        for (out_port_num = startoutport; out_port_num < endoutport;
>             out_port_num++)
>
> nothing will be done. Right?

Right; fixed in your patch.

>
>> +     if (attr_mod & 0x20000) {
>> +             startinport = ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info) ? 0 : 1;
>> +             endinport = osm_node_get_num_physp(p_node);
>> +     } else
>> +             endinport = startinport = in_port_num;
>
> Ditto with 'inport'.

Ditto.

-- Hal

> Sasha
>
>> +
>> +     for (out_port_num = startoutport; out_port_num < endoutport;
>> +          out_port_num++) {
>> +             p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
>> +             for (in_port_num = startinport; in_port_num < endinport;
>> +                  in_port_num++)
>> +                     osm_physp_set_slvl_tbl(p_physp, p_slvl_tbl, in_port_num);
>> +     }
>>
>>  Exit:
>>       cl_plock_release(sm->p_lock);
>> diff --git a/opensm/opensm/osm_subnet.c b/opensm/opensm/osm_subnet.c
>> index 09029db..103b5c6 100644
>> --- a/opensm/opensm/osm_subnet.c
>> +++ b/opensm/opensm/osm_subnet.c
>> @@ -354,6 +354,7 @@ static const opt_rec_t opt_tbl[] = {
>>       { "daemon", OPT_OFFSET(daemon), opts_parse_boolean, NULL, 0 },
>>       { "sm_inactive", OPT_OFFSET(sm_inactive), opts_parse_boolean, NULL, 1 },
>>       { "babbling_port_policy", OPT_OFFSET(babbling_port_policy), opts_parse_boolean, NULL, 1 },
>> +     { "use_optimized_slvl", OPT_OFFSET(use_optimized_slvl), opts_parse_boolean, NULL, 1 },
>>  #ifdef ENABLE_OSM_PERF_MGR
>>       { "perfmgr", OPT_OFFSET(perfmgr), opts_parse_boolean, NULL, 0 },
>>       { "perfmgr_redir", OPT_OFFSET(perfmgr_redir), opts_parse_boolean, NULL, 0 },
>> @@ -713,6 +714,7 @@ void osm_subn_set_default_opt(IN osm_subn_opt_t * p_opt)
>>       p_opt->daemon = FALSE;
>>       p_opt->sm_inactive = FALSE;
>>       p_opt->babbling_port_policy = FALSE;
>> +     p_opt->use_optimized_slvl = FALSE;
>>  #ifdef ENABLE_OSM_PERF_MGR
>>       p_opt->perfmgr = FALSE;
>>       p_opt->perfmgr_redir = TRUE;
>> @@ -1499,10 +1501,13 @@ int osm_subn_output_conf(FILE *out, IN osm_subn_opt_t * p_opts)
>>               "# SM Inactive\n"
>>               "sm_inactive %s\n\n"
>>               "# Babbling Port Policy\n"
>> -             "babbling_port_policy %s\n\n",
>> +             "babbling_port_policy %s\n\n"
>> +             "# Use Optimized SLtoVLMapping programming if supported by device\n"
>> +             "use_optimized_slvl %s\n\n",
>>               p_opts->daemon ? "TRUE" : "FALSE",
>>               p_opts->sm_inactive ? "TRUE" : "FALSE",
>> -             p_opts->babbling_port_policy ? "TRUE" : "FALSE");
>> +             p_opts->babbling_port_policy ? "TRUE" : "FALSE",
>> +             p_opts->use_optimized_slvl ? "TRUE" : "FALSE");
>>
>>  #ifdef ENABLE_OSM_PERF_MGR
>>       fprintf(out,
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
  2009-12-29 15:07     ` [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network Sasha Khapyorsky
@ 2009-12-30 16:09       ` Hal Rosenstock
       [not found]         ` <f0e08f230912300809l7c794daat9490c64a2cf63525-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2009-12-30 16:09 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Tue, Dec 29, 2009 at 10:07 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
>
> Verify that port number values received from network are in range and
> valid for access. Report error otherwise.

Won't this spam the OpenSM log ?

> Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
> ---
>  opensm/opensm/osm_slvl_map_rcv.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
> index b35d867..6229db9 100644
> --- a/opensm/opensm/osm_slvl_map_rcv.c
> +++ b/opensm/opensm/osm_slvl_map_rcv.c
> @@ -104,8 +104,17 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>
>        /* in case of a non switch node the attr modifier should be ignored */
>        if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
> +               unsigned num_ports = osm_node_get_num_physp(p_node) - 1;
>                out_port_num = cl_ntoh32(p_smp->attr_mod) & 0xff;
>                in_port_num = (cl_ntoh32(p_smp->attr_mod) >> 8) & 0xff;
> +               if (in_port_num > num_ports || out_port_num > num_ports) {
> +                       OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 2C07"
> +                               "Invalid attribute modifier 0x%x reveived in"

           ^^^^^^^^

            typo

-- Hal

> +                               " response from switch 0x%" PRIx64 "\n",
> +                               cl_ntoh32(p_smp->attr_mod),
> +                               cl_ntoh64(node_guid));
> +                       goto Exit;
> +               }
>                p_physp = osm_node_get_physp_ptr(p_node, out_port_num);
>        } else {
>                p_physp = p_port->p_physp;
> --
> 1.6.6.rc4
>
> --
> 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
>
--
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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
       [not found]         ` <f0e08f230912300809l7c794daat9490c64a2cf63525-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2009-12-30 16:27           ` Sasha Khapyorsky
  2009-12-30 16:56             ` Hal Rosenstock
  2010-01-04 19:31             ` Hal Rosenstock
  0 siblings, 2 replies; 37+ messages in thread
From: Sasha Khapyorsky @ 2009-12-30 16:27 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 11:09 Wed 30 Dec     , Hal Rosenstock wrote:
> On Tue, Dec 29, 2009 at 10:07 AM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
> >
> > Verify that port number values received from network are in range and
> > valid for access. Report error otherwise.
> 
> Won't this spam the OpenSM log ?

Yes, unfortunately. But unlike SA queries, etc., this is response to
OpenSM originated request and messing attribute modifier there is more
critical there than just invalid query from some client.

However basically I am sharing your concerns. Maybe we need a separate
log level for things like that (OSM_LOG_WARNING or so).

> > Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
> > ---
> >  opensm/opensm/osm_slvl_map_rcv.c |    9 +++++++++
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
> > index b35d867..6229db9 100644
> > --- a/opensm/opensm/osm_slvl_map_rcv.c
> > +++ b/opensm/opensm/osm_slvl_map_rcv.c
> > @@ -104,8 +104,17 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
> >
> >        /* in case of a non switch node the attr modifier should be ignored */
> >        if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
> > +               unsigned num_ports = osm_node_get_num_physp(p_node) - 1;
> >                out_port_num = cl_ntoh32(p_smp->attr_mod) & 0xff;
> >                in_port_num = (cl_ntoh32(p_smp->attr_mod) >> 8) & 0xff;
> > +               if (in_port_num > num_ports || out_port_num > num_ports) {
> > +                       OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 2C07"
> > +                               "Invalid attribute modifier 0x%x reveived in"
> 
>            ^^^^^^^^
> 
>             typo

Thanks for finding. I'm fixing this.

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
  2009-12-30 16:27           ` Sasha Khapyorsky
@ 2009-12-30 16:56             ` Hal Rosenstock
       [not found]               ` <f0e08f230912300856v188d5900u9d82c94f5f9ae4a6-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2010-01-04 19:31             ` Hal Rosenstock
  1 sibling, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2009-12-30 16:56 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Dec 30, 2009 at 11:27 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 11:09 Wed 30 Dec     , Hal Rosenstock wrote:
>> On Tue, Dec 29, 2009 at 10:07 AM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
>> >
>> > Verify that port number values received from network are in range and
>> > valid for access. Report error otherwise.
>>
>> Won't this spam the OpenSM log ?
>
> Yes, unfortunately. But unlike SA queries, etc., this is response to
> OpenSM originated request and messing attribute modifier there is more
> critical there than just invalid query from some client.

Yes but why would SMA mess up attribute modifier though ? So is this
just detecting OpenSM bug with AM ?

-- Hal

> However basically I am sharing your concerns. Maybe we need a separate
> log level for things like that (OSM_LOG_WARNING or so).
>
>> > Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
>> > ---
>> >  opensm/opensm/osm_slvl_map_rcv.c |    9 +++++++++
>> >  1 files changed, 9 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
>> > index b35d867..6229db9 100644
>> > --- a/opensm/opensm/osm_slvl_map_rcv.c
>> > +++ b/opensm/opensm/osm_slvl_map_rcv.c
>> > @@ -104,8 +104,17 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>> >
>> >        /* in case of a non switch node the attr modifier should be ignored */
>> >        if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
>> > +               unsigned num_ports = osm_node_get_num_physp(p_node) - 1;
>> >                out_port_num = cl_ntoh32(p_smp->attr_mod) & 0xff;
>> >                in_port_num = (cl_ntoh32(p_smp->attr_mod) >> 8) & 0xff;
>> > +               if (in_port_num > num_ports || out_port_num > num_ports) {
>> > +                       OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 2C07"
>> > +                               "Invalid attribute modifier 0x%x reveived in"
>>
>>            ^^^^^^^^
>>
>>             typo
>
> Thanks for finding. I'm fixing this.
>
> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
       [not found]               ` <f0e08f230912300856v188d5900u9d82c94f5f9ae4a6-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2009-12-30 19:43                 ` Sasha Khapyorsky
  2009-12-31 14:48                   ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2009-12-30 19:43 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 11:56 Wed 30 Dec     , Hal Rosenstock wrote:
> 
> Yes but why would SMA mess up attribute modifier though ?

How could I know? :)

> So is this
> just detecting OpenSM bug with AM ?

It is not for OpenSM internal bugs detecting. It is "don't trust to
information from a network", in this specific case attribute modifier
elements are used as pointers indexes (sl2vl tables), so OpenSM don't
need to crash due to broken SMAs or other sort of attack.

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
  2009-12-30 19:43                 ` Sasha Khapyorsky
@ 2009-12-31 14:48                   ` Hal Rosenstock
       [not found]                     ` <f0e08f230912310648m4b03b9bdq92394a61866f0e26-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2009-12-31 14:48 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Dec 30, 2009 at 2:43 PM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 11:56 Wed 30 Dec     , Hal Rosenstock wrote:
>>
>> Yes but why would SMA mess up attribute modifier though ?
>
> How could I know? :)
>
>> So is this
>> just detecting OpenSM bug with AM ?
>
> It is not for OpenSM internal bugs detecting. It is "don't trust to
> information from a network", in this specific case attribute modifier
> elements are used as pointers indexes (sl2vl tables),

Yes, I noticed that but...

> so OpenSM don't need to crash due to broken SMAs or other sort of attack.

I thought this was only done when these issues are actually found and
in general we are relying on compliance. Is there a specific known
issue here ?

If we are worrying about this as a general principle, there are more
places to bullet proof like this. I am wondering about why the
exception here.

-- Hal

>
> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
       [not found]                     ` <f0e08f230912310648m4b03b9bdq92394a61866f0e26-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-03 17:02                       ` Sasha Khapyorsky
  2010-01-04 19:12                         ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-03 17:02 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 09:48 Thu 31 Dec     , Hal Rosenstock wrote:
> 
> I thought this was only done when these issues are actually found and
> in general we are relying on compliance.

OpenSM can rely on compliance in many cases, but it cannot crash (or
become nonoperational in any other way) relying on compliance.

> Is there a specific known
> issue here ?
> 
> If we are worrying about this as a general principle, there are more
> places to bullet proof like this.

Likely so and it would be good to fix all of them.

> I am wondering about why the
> exception here.

It is not exception. I found this one, so fixed.

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check
       [not found] ` <20091201194110.GA26753-Wuw85uim5zDR7s880joybQ@public.gmane.org>
                     ` (2 preceding siblings ...)
  2009-12-29 16:29   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
@ 2010-01-04 17:01   ` Sasha Khapyorsky
  2010-01-05 11:10     ` [PATCH] opensm/osm_qos.c: split switch external and end ports setup Sasha Khapyorsky
  2010-01-07 13:38     ` [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check Hal Rosenstock
  2010-01-05 11:18   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
  2010-01-06 19:13   ` Sasha Khapyorsky
  5 siblings, 2 replies; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-04 17:01 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Hal Rosenstock


The IBA 1.2.1 states (Vol.1, p.840, Table XX, explanation (b)) that
SL2VL mapping capability for switch external ports must be indicated on
its port 0 PortInfo:CapMask. So it lets us to unify this capability
check over all types of IB nodes.

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_qos.c |   22 +++++++---------------
 1 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
index 08f9a60..afeaa11 100644
--- a/opensm/opensm/osm_qos.c
+++ b/opensm/opensm/osm_qos.c
@@ -194,23 +194,15 @@ static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
 {
 	ib_api_status_t status;
 	uint8_t i, num_ports;
-	osm_physp_t *p_physp;
+	ib_port_info_t *pi = &p_port->p_physp->port_info;
+
+	if (!(pi->capability_mask & IB_PORT_CAP_HAS_SL_MAP))
+		return IB_SUCCESS;
 
-	if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH) {
-		if (ib_port_info_get_vl_cap(&p->port_info) == 1) {
-			/* Check port 0's capability mask */
-			p_physp = p_port->p_physp;
-			if (!
-			    (p_physp->port_info.
-			     capability_mask & IB_PORT_CAP_HAS_SL_MAP))
-				return IB_SUCCESS;
-		}
+	if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH)
 		num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
-	} else {
-		if (!(p->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
-			return IB_SUCCESS;
+	else
 		num_ports = 1;
-	}
 
 	for (i = 0; i < num_ports; i++) {
 		status = sl2vl_update_table(sm, p, i, port_num, force_update,
-- 
1.6.6

--
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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
  2010-01-03 17:02                       ` Sasha Khapyorsky
@ 2010-01-04 19:12                         ` Hal Rosenstock
       [not found]                           ` <f0e08f231001041112k4cb7742bwb5ae7cade8704082-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-04 19:12 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Sun, Jan 3, 2010 at 12:02 PM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 09:48 Thu 31 Dec     , Hal Rosenstock wrote:
>>
>> I thought this was only done when these issues are actually found and
>> in general we are relying on compliance.
>
> OpenSM can rely on compliance in many cases, but it cannot crash (or
> become nonoperational in any other way) relying on compliance.

I think that's fine as a general principle but isn't practical. There
are many cases where OpenSM relies on SMA/PMA/SA client compliance
(without such checking/overhead).

>> Is there a specific known
>> issue here ?
>>
>> If we are worrying about this as a general principle, there are more
>> places to bullet proof like this.
>
> Likely so and it would be good to fix all of them.

Have you done a code review/inspection for such cases ?

>> I am wondering about why the
>> exception here.
>
> It is not exception.

I think there are more unprotected cases than protected ones.

-- Hal

> I found this one, so fixed.

> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
  2009-12-30 16:27           ` Sasha Khapyorsky
  2009-12-30 16:56             ` Hal Rosenstock
@ 2010-01-04 19:31             ` Hal Rosenstock
  1 sibling, 0 replies; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-04 19:31 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Wed, Dec 30, 2009 at 11:27 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 11:09 Wed 30 Dec     , Hal Rosenstock wrote:
>> On Tue, Dec 29, 2009 at 10:07 AM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
>> >
>> > Verify that port number values received from network are in range and
>> > valid for access. Report error otherwise.
>>
>> Won't this spam the OpenSM log ?
>
> Yes, unfortunately. But unlike SA queries, etc., this is response to
> OpenSM originated request and messing attribute modifier there is more
> critical there than just invalid query from some client.

I wasn't referring to just SA queries.

I don't think this is consistent with your treatment of other proposed
error messages.

> However basically I am sharing your concerns. Maybe we need a separate
> log level for things like that (OSM_LOG_WARNING or so).

I think the logging needs more than just another level.

-- Hal

>> > Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
>> > ---
>> >  opensm/opensm/osm_slvl_map_rcv.c |    9 +++++++++
>> >  1 files changed, 9 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/opensm/opensm/osm_slvl_map_rcv.c b/opensm/opensm/osm_slvl_map_rcv.c
>> > index b35d867..6229db9 100644
>> > --- a/opensm/opensm/osm_slvl_map_rcv.c
>> > +++ b/opensm/opensm/osm_slvl_map_rcv.c
>> > @@ -104,8 +104,17 @@ void osm_slvl_rcv_process(IN void *context, IN void *p_data)
>> >
>> >        /* in case of a non switch node the attr modifier should be ignored */
>> >        if (osm_node_get_type(p_node) == IB_NODE_TYPE_SWITCH) {
>> > +               unsigned num_ports = osm_node_get_num_physp(p_node) - 1;
>> >                out_port_num = cl_ntoh32(p_smp->attr_mod) & 0xff;
>> >                in_port_num = (cl_ntoh32(p_smp->attr_mod) >> 8) & 0xff;
>> > +               if (in_port_num > num_ports || out_port_num > num_ports) {
>> > +                       OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 2C07"
>> > +                               "Invalid attribute modifier 0x%x reveived in"
>>
>>            ^^^^^^^^
>>
>>             typo
>
> Thanks for finding. I'm fixing this.
>
> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network
       [not found]                           ` <f0e08f231001041112k4cb7742bwb5ae7cade8704082-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-04 20:03                             ` Sasha Khapyorsky
  0 siblings, 0 replies; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-04 20:03 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 14:12 Mon 04 Jan     , Hal Rosenstock wrote:
> >>
> >> If we are worrying about this as a general principle, there are more
> >> places to bullet proof like this.
> >
> > Likely so and it would be good to fix all of them.
> 
> Have you done a code review/inspection for such cases ?

No, I've never hunted this specially, perhaps fixed couple of such
occasionally (like this one).

> I think there are more unprotected cases than protected ones.

Have you done a code review/inspection?

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH] opensm/osm_qos.c: split switch external and end ports setup
  2010-01-04 17:01   ` [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check Sasha Khapyorsky
@ 2010-01-05 11:10     ` Sasha Khapyorsky
  2010-01-07 13:40       ` Hal Rosenstock
  2010-01-07 13:38     ` [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check Hal Rosenstock
  1 sibling, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-05 11:10 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Hal Rosenstock, Eli Dorfman


This splits QoS related port parameters setup over switch external ports
and end ports. Such separation leaves us with simpler code and saves
some repeated flows in case of switch external ports (actually required
per switch and not per port).

Another advantages: Optimized Sl2VL Mapping procedure can be implemented
easier using this model. A low level port QoS related parameters setup
infrastructure is ready for supporting per port QoS related configuration
(which hopefully will be implemented some days).

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_qos.c |  137 +++++++++++++++++++++--------------------------
 1 files changed, 61 insertions(+), 76 deletions(-)

diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
index afeaa11..f54e995 100644
--- a/opensm/opensm/osm_qos.c
+++ b/opensm/opensm/osm_qos.c
@@ -77,6 +77,7 @@ static ib_api_status_t vlarb_update_table_block(osm_sm_t * sm,
 	osm_madw_context_t context;
 	uint32_t attr_mod;
 	unsigned vl_mask, i;
+	ib_api_status_t status;
 
 	vl_mask = (1 << (ib_port_info_get_op_vls(&p->port_info) - 1)) - 1;
 
@@ -96,10 +97,17 @@ static ib_api_status_t vlarb_update_table_block(osm_sm_t * sm,
 	context.vla_context.set_method = TRUE;
 	attr_mod = ((block_num + 1) << 16) | port_num;
 
-	return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
-			   (uint8_t *) & block, sizeof(block),
-			   IB_MAD_ATTR_VL_ARBITRATION, cl_hton32(attr_mod),
-			   CL_DISP_MSGID_NONE, &context);
+	status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
+			     (uint8_t *) & block, sizeof(block),
+			     IB_MAD_ATTR_VL_ARBITRATION, cl_hton32(attr_mod),
+			     CL_DISP_MSGID_NONE, &context);
+	if (status != IB_SUCCESS)
+		OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 6202 : "
+			"failed to update VLArbitration tables "
+			"for port %" PRIx64 " block %u\n",
+			cl_ntoh64(p->port_guid), block_num);
+
+	return status;
 }
 
 static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
@@ -157,6 +165,7 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
 	ib_slvl_table_t tbl, *p_tbl;
 	osm_node_t *p_node = osm_physp_get_node_ptr(p);
 	uint32_t attr_mod;
+	ib_api_status_t status;
 	unsigned vl_mask;
 	uint8_t vl1, vl2;
 	int i;
@@ -181,70 +190,65 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
 	context.slvl_context.port_guid = osm_physp_get_port_guid(p);
 	context.slvl_context.set_method = TRUE;
 	attr_mod = in_port << 8 | out_port;
-	return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
-			   (uint8_t *) & tbl, sizeof(tbl),
-			   IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
-			   CL_DISP_MSGID_NONE, &context);
+	status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
+			     (uint8_t *) & tbl, sizeof(tbl),
+			     IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
+			     CL_DISP_MSGID_NONE, &context);
+	if (status != IB_SUCCESS)
+		OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 6203 : "
+			"failed to update SL2VLMapping tables "
+			"for port %" PRIx64 "i, attr_mod 0x%x\n",
+			cl_ntoh64(p->port_guid), attr_mod);
+	return status;
 }
 
-static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
-				    osm_physp_t * p, uint8_t port_num,
-				    unsigned force_update,
-				    const struct qos_config *qcfg)
+static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
+			      const struct qos_config *qcfg)
 {
-	ib_api_status_t status;
-	uint8_t i, num_ports;
-	ib_port_info_t *pi = &p_port->p_physp->port_info;
-
-	if (!(pi->capability_mask & IB_PORT_CAP_HAS_SL_MAP))
-		return IB_SUCCESS;
+	osm_physp_t *p0, *p;
+	unsigned force_update;
+	unsigned num_ports = osm_node_get_num_physp(node);
+	int ret = 0;
+	unsigned i, j;
 
-	if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH)
-		num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
-	else
-		num_ports = 1;
+	for (i = 1; i < num_ports; i++) {
+		p = osm_node_get_physp_ptr(node, i);
+		force_update = p->need_update || sm->p_subn->need_update;
+		p->vl_high_limit = qcfg->vl_high_limit;
+		if (vlarb_update(sm, p, p->port_num, force_update, qcfg))
+			ret = -1;
+	}
 
-	for (i = 0; i < num_ports; i++) {
-		status = sl2vl_update_table(sm, p, i, port_num, force_update,
-					    &qcfg->sl2vl);
-		if (status != IB_SUCCESS)
-			return status;
+	p0 = osm_node_get_physp_ptr(node, 0);
+	if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
+		return ret;
+
+	for (i = 1; i < num_ports; i++) {
+		p = osm_node_get_physp_ptr(node, i);
+		force_update = p->need_update || sm->p_subn->need_update;
+		for (j = 0; j < num_ports; j++)
+			if (sl2vl_update_table(sm, p, i, j, force_update,
+					       &qcfg->sl2vl))
+				ret = -1;
 	}
 
-	return IB_SUCCESS;
+	return ret;
 }
 
-static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
-			   osm_port_t * p_port, osm_physp_t * p,
-			   uint8_t port_num, unsigned force_update,
-			   const struct qos_config *qcfg)
+static int qos_endport_setup(osm_sm_t * sm, osm_physp_t * p,
+			     const struct qos_config *qcfg)
 {
-	ib_api_status_t status;
+	unsigned force_update = p->need_update || sm->p_subn->need_update;
 
-	/* OpVLs should be ok at this moment - just use it */
-
-	/* setup VL high limit on the physp later to be updated by link mgr */
 	p->vl_high_limit = qcfg->vl_high_limit;
-
-	/* setup VLArbitration */
-	status = vlarb_update(sm, p, port_num, force_update, qcfg);
-	if (status != IB_SUCCESS) {
-		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6202 : "
-			"failed to update VLArbitration tables "
-			"for port %" PRIx64 " #%d\n",
-			cl_ntoh64(p->port_guid), port_num);
+	if (vlarb_update(sm, p, 0, force_update, qcfg))
 		return -1;
-	}
 
-	/* setup SL2VL tables */
-	status = sl2vl_update(sm, p_port, p, port_num, force_update, qcfg);
-	if (status != IB_SUCCESS) {
-		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6203 : "
-			"failed to update SL2VLMapping tables "
-			"for port %" PRIx64 " #%d\n",
-			cl_ntoh64(p->port_guid), port_num);
+	if (!(p->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
+		return 0;
+
+	if (sl2vl_update_table(sm, p, 0, 0, force_update, &qcfg->sl2vl))
 		return -1;
-	}
 
 	return 0;
 }
@@ -256,12 +260,8 @@ int osm_qos_setup(osm_opensm_t * p_osm)
 	cl_qmap_t *p_tbl;
 	cl_map_item_t *p_next;
 	osm_port_t *p_port;
-	uint32_t num_physp;
-	osm_physp_t *p_physp;
 	osm_node_t *p_node;
-	unsigned force_update;
 	int ret = 0;
-	uint8_t i;
 
 	if (!p_osm->subn.opt.qos)
 		return 0;
@@ -290,18 +290,9 @@ int osm_qos_setup(osm_opensm_t * p_osm)
 
 		p_node = p_port->p_node;
 		if (p_node->sw) {
-			num_physp = osm_node_get_num_physp(p_node);
-			for (i = 1; i < num_physp; i++) {
-				p_physp = osm_node_get_physp_ptr(p_node, i);
-				if (!p_physp)
-					continue;
-				force_update = p_physp->need_update ||
-				    p_osm->subn.need_update;
-				if (qos_physp_setup(&p_osm->log, &p_osm->sm,
-						    p_port, p_physp, i,
-						    force_update, &swe_config))
-					ret = -1;
-			}
+			if (qos_extports_setup(&p_osm->sm, p_node, &swe_config))
+				ret = -1;
+
 			/* skip base port 0 */
 			if (!ib_switch_info_is_enhanced_port0
 			    (&p_node->sw->switch_info))
@@ -313,13 +304,7 @@ int osm_qos_setup(osm_opensm_t * p_osm)
 		else
 			cfg = &ca_config;
 
-		p_physp = p_port->p_physp;
-		if (!p_physp)
-			continue;
-
-		force_update = p_physp->need_update || p_osm->subn.need_update;
-		if (qos_physp_setup(&p_osm->log, &p_osm->sm, p_port, p_physp,
-				    0, force_update, cfg))
+		if (qos_endport_setup(&p_osm->sm, p_port->p_physp, cfg))
 			ret = -1;
 	}
 
-- 
1.6.6

--
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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
       [not found] ` <20091201194110.GA26753-Wuw85uim5zDR7s880joybQ@public.gmane.org>
                     ` (3 preceding siblings ...)
  2010-01-04 17:01   ` [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check Sasha Khapyorsky
@ 2010-01-05 11:18   ` Sasha Khapyorsky
  2010-01-05 11:23     ` Sasha Khapyorsky
  2010-01-07 13:34     ` Hal Rosenstock
  2010-01-06 19:13   ` Sasha Khapyorsky
  5 siblings, 2 replies; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-05 11:18 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Hi Hal,

On 14:41 Tue 01 Dec     , Hal Rosenstock wrote:
> 
> Optimized SLtoVLMappingTable programming reduces the number of MADs
> needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
> SLtoVLMappingTable.
> 
> Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---

[snip]

> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> index 08f9a60..617a86e 100644
> --- a/opensm/opensm/osm_qos.c
> +++ b/opensm/opensm/osm_qos.c

[snip]

> @@ -290,6 +306,44 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>  	/* read QoS policy config file */
>  	osm_qos_parse_policy_file(&p_osm->subn);
>  
> +	/* loop on switches that support optimized SL2VL programming first */
> +	p_tbl = &p_osm->subn.sw_guid_tbl;
> +	p_next = cl_qmap_head(p_tbl);
> +	while (p_next != cl_qmap_end(p_tbl)) {
> +		p_sw = (osm_switch_t *) p_next;
> +		p_next = cl_qmap_next(p_next);
> +
> +		if (ib_switch_info_get_opt_sl2vlmapping(&p_sw->switch_info) &&
> +		    p_osm->subn.opt.use_optimized_slvl) {
> +			p_physp = osm_node_get_physp_ptr(p_sw->p_node, 1);
> +			num_physp = osm_node_get_num_physp(p_sw->p_node);
> +			force_update = p_osm->subn.need_update;
> +			for (i = 1; i < num_physp; i++) {
> +				p_physp = osm_node_get_physp_ptr(p_sw->p_node, i);
> +				if (!p_physp)
> +					continue;
> +				if (vlarb_physp_setup(&p_osm->sm, p_physp, i,
> +						      p_physp->need_update ||
> +						      p_osm->subn.need_update,
> +						      &swe_config))
> +					ret = -1;
> +				force_update |= p_physp->need_update;
> +			}
> +			if (sl2vl_update(&p_osm->sm,
> +					 osm_node_get_physp_ptr(p_sw->p_node, 0),
> +					 p_physp, i, 1, force_update,

At this point i = num_physp, which is an invalid switch port number,
right?

Sasha


> +					 &swe_config)) {
> +				OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 6204: "
> +					"failed to update optimized SL2VLMapping"
> +					" tables for port %" PRIx64 " #%d\n",
> +					cl_ntoh64(p_physp->port_guid), i);
> +				ret = -1;
> +			}
> +		}
> +	}
> +
> +	/* now, loop on ports skipping the external ports of switches
> +	   that support optimized SL2VL programming */
>  	p_tbl = &p_osm->subn.port_guid_tbl;
>  	p_next = cl_qmap_head(p_tbl);
>  	while (p_next != cl_qmap_end(p_tbl)) {
--
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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
  2010-01-05 11:18   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
@ 2010-01-05 11:23     ` Sasha Khapyorsky
  2010-01-07 13:35       ` Hal Rosenstock
  2010-01-07 13:34     ` Hal Rosenstock
  1 sibling, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-05 11:23 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 13:18 Tue 05 Jan     , Sasha Khapyorsky wrote:
> > 
> > Optimized SLtoVLMappingTable programming reduces the number of MADs
> > needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
> > SLtoVLMappingTable.
> > 
> > Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> 
> [snip]
> 
> > diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> > index 08f9a60..617a86e 100644
> > --- a/opensm/opensm/osm_qos.c
> > +++ b/opensm/opensm/osm_qos.c
> 

After rebasing this part over recently posted patches it becomes:

diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
index f54e995..6bbbfa2 100644
--- a/opensm/opensm/osm_qos.c
+++ b/opensm/opensm/osm_qos.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2006-2009 Voltaire, Inc. All rights reserved.
+ * Copyright (c) 2009 HNR Consulting. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -157,14 +158,13 @@ static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
 }
 
 static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
-					  uint8_t in_port, uint8_t out_port,
+					  uint8_t in_port, uint32_t attr_mod,
 					  unsigned force_update,
 					  const ib_slvl_table_t * sl2vl_table)
 {
 	osm_madw_context_t context;
 	ib_slvl_table_t tbl, *p_tbl;
 	osm_node_t *p_node = osm_physp_get_node_ptr(p);
-	uint32_t attr_mod;
 	ib_api_status_t status;
 	unsigned vl_mask;
 	uint8_t vl1, vl2;
@@ -189,7 +189,6 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
 	context.slvl_context.node_guid = osm_node_get_node_guid(p_node);
 	context.slvl_context.port_guid = osm_physp_get_port_guid(p);
 	context.slvl_context.set_method = TRUE;
-	attr_mod = in_port << 8 | out_port;
 	status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
 			     (uint8_t *) & tbl, sizeof(tbl),
 			     IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
@@ -223,12 +222,20 @@ static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
 	if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
 		return ret;
 
+	if (ib_switch_info_get_opt_sl2vlmapping(&node->sw->switch_info) &&
+	    sm->p_subn->opt.use_optimized_slvl) {
+		p = osm_node_get_physp_ptr(node, 1);
+		force_update = p->need_update || sm->p_subn->need_update;
+		return sl2vl_update_table(sm, p, 1, 0x30000, force_update,
+					  &qcfg->sl2vl);
+	}
+
 	for (i = 1; i < num_ports; i++) {
 		p = osm_node_get_physp_ptr(node, i);
 		force_update = p->need_update || sm->p_subn->need_update;
 		for (j = 0; j < num_ports; j++)
-			if (sl2vl_update_table(sm, p, i, j, force_update,
-					       &qcfg->sl2vl))
+			if (sl2vl_update_table(sm, p, i, i << 8 | j,
+					       force_update, &qcfg->sl2vl))
 				ret = -1;
 	}


, does it look fine for you?

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

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
       [not found] ` <20091201194110.GA26753-Wuw85uim5zDR7s880joybQ@public.gmane.org>
                     ` (4 preceding siblings ...)
  2010-01-05 11:18   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
@ 2010-01-06 19:13   ` Sasha Khapyorsky
  5 siblings, 0 replies; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-06 19:13 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 14:41 Tue 01 Dec     , Hal Rosenstock wrote:
> 
> Optimized SLtoVLMappingTable programming reduces the number of MADs
> needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
> SLtoVLMappingTable.
> 
> Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Rebased and 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
  2010-01-05 11:18   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
  2010-01-05 11:23     ` Sasha Khapyorsky
@ 2010-01-07 13:34     ` Hal Rosenstock
  1 sibling, 0 replies; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-07 13:34 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Hi Sasha,

On Tue, Jan 5, 2010 at 6:18 AM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
> Hi Hal,
>
> On 14:41 Tue 01 Dec     , Hal Rosenstock wrote:
>>
>> Optimized SLtoVLMappingTable programming reduces the number of MADs
>> needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
>> SLtoVLMappingTable.
>>
>> Signed-off-by: Hal Rosenstock <hal.rosenstock@gmail.com>
>> ---
>
> [snip]
>
>> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
>> index 08f9a60..617a86e 100644
>> --- a/opensm/opensm/osm_qos.c
>> +++ b/opensm/opensm/osm_qos.c
>
> [snip]
>
>> @@ -290,6 +306,44 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>>       /* read QoS policy config file */
>>       osm_qos_parse_policy_file(&p_osm->subn);
>>
>> +     /* loop on switches that support optimized SL2VL programming first */
>> +     p_tbl = &p_osm->subn.sw_guid_tbl;
>> +     p_next = cl_qmap_head(p_tbl);
>> +     while (p_next != cl_qmap_end(p_tbl)) {
>> +             p_sw = (osm_switch_t *) p_next;
>> +             p_next = cl_qmap_next(p_next);
>> +
>> +             if (ib_switch_info_get_opt_sl2vlmapping(&p_sw->switch_info) &&
>> +                 p_osm->subn.opt.use_optimized_slvl) {
>> +                     p_physp = osm_node_get_physp_ptr(p_sw->p_node, 1);
>> +                     num_physp = osm_node_get_num_physp(p_sw->p_node);
>> +                     force_update = p_osm->subn.need_update;
>> +                     for (i = 1; i < num_physp; i++) {
>> +                             p_physp = osm_node_get_physp_ptr(p_sw->p_node, i);
>> +                             if (!p_physp)
>> +                                     continue;
>> +                             if (vlarb_physp_setup(&p_osm->sm, p_physp, i,
>> +                                                   p_physp->need_update ||
>> +                                                   p_osm->subn.need_update,
>> +                                                   &swe_config))
>> +                                     ret = -1;
>> +                             force_update |= p_physp->need_update;
>> +                     }
>> +                     if (sl2vl_update(&p_osm->sm,
>> +                                      osm_node_get_physp_ptr(p_sw->p_node, 0),
>> +                                      p_physp, i, 1, force_update,
>
> At this point i = num_physp, which is an invalid switch port number,
> right?

Yes, but in sl2vl_update_table output port num is not used when
optimize is 1 which is the case here.

-- Hal

> Sasha
>
>
>> +                                      &swe_config)) {
>> +                             OSM_LOG(&p_osm->log, OSM_LOG_ERROR, "ERR 6204: "
>> +                                     "failed to update optimized SL2VLMapping"
>> +                                     " tables for port %" PRIx64 " #%d\n",
>> +                                     cl_ntoh64(p_physp->port_guid), i);
>> +                             ret = -1;
>> +                     }
>> +             }
>> +     }
>> +
>> +     /* now, loop on ports skipping the external ports of switches
>> +        that support optimized SL2VL programming */
>>       p_tbl = &p_osm->subn.port_guid_tbl;
>>       p_next = cl_qmap_head(p_tbl);
>>       while (p_next != cl_qmap_end(p_tbl)) {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
  2010-01-05 11:23     ` Sasha Khapyorsky
@ 2010-01-07 13:35       ` Hal Rosenstock
       [not found]         ` <f0e08f231001070535l382f1d54t7f6290607e52b196-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-07 13:35 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Tue, Jan 5, 2010 at 6:23 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 13:18 Tue 05 Jan     , Sasha Khapyorsky wrote:
>> >
>> > Optimized SLtoVLMappingTable programming reduces the number of MADs
>> > needed from O(n**2) to O(n). See IBA 1.2.1 vol 1 p. 843 14.2.5.8
>> > SLtoVLMappingTable.
>> >
>> > Signed-off-by: Hal Rosenstock <hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> > ---
>>
>> [snip]
>>
>> > diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
>> > index 08f9a60..617a86e 100644
>> > --- a/opensm/opensm/osm_qos.c
>> > +++ b/opensm/opensm/osm_qos.c
>>
>
> After rebasing this part over recently posted patches it becomes:
>
> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> index f54e995..6bbbfa2 100644
> --- a/opensm/opensm/osm_qos.c
> +++ b/opensm/opensm/osm_qos.c
> @@ -1,5 +1,6 @@
>  /*
>  * Copyright (c) 2006-2009 Voltaire, Inc. All rights reserved.
> + * Copyright (c) 2009 HNR Consulting. All rights reserved.
>  *
>  * This software is available to you under a choice of one of two
>  * licenses.  You may choose to be licensed under the terms of the GNU
> @@ -157,14 +158,13 @@ static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
>  }
>
>  static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
> -                                         uint8_t in_port, uint8_t out_port,
> +                                         uint8_t in_port, uint32_t attr_mod,
>                                          unsigned force_update,
>                                          const ib_slvl_table_t * sl2vl_table)
>  {
>        osm_madw_context_t context;
>        ib_slvl_table_t tbl, *p_tbl;
>        osm_node_t *p_node = osm_physp_get_node_ptr(p);
> -       uint32_t attr_mod;
>        ib_api_status_t status;
>        unsigned vl_mask;
>        uint8_t vl1, vl2;
> @@ -189,7 +189,6 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>        context.slvl_context.node_guid = osm_node_get_node_guid(p_node);
>        context.slvl_context.port_guid = osm_physp_get_port_guid(p);
>        context.slvl_context.set_method = TRUE;
> -       attr_mod = in_port << 8 | out_port;
>        status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
>                             (uint8_t *) & tbl, sizeof(tbl),
>                             IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
> @@ -223,12 +222,20 @@ static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
>        if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>                return ret;
>
> +       if (ib_switch_info_get_opt_sl2vlmapping(&node->sw->switch_info) &&
> +           sm->p_subn->opt.use_optimized_slvl) {
> +               p = osm_node_get_physp_ptr(node, 1);
> +               force_update = p->need_update || sm->p_subn->need_update;
> +               return sl2vl_update_table(sm, p, 1, 0x30000, force_update,
> +                                         &qcfg->sl2vl);
> +       }
> +
>        for (i = 1; i < num_ports; i++) {
>                p = osm_node_get_physp_ptr(node, i);
>                force_update = p->need_update || sm->p_subn->need_update;
>                for (j = 0; j < num_ports; j++)
> -                       if (sl2vl_update_table(sm, p, i, j, force_update,
> -                                              &qcfg->sl2vl))
> +                       if (sl2vl_update_table(sm, p, i, i << 8 | j,
> +                                              force_update, &qcfg->sl2vl))
>                                ret = -1;
>        }
>
>
> , does it look fine for you?

In the optimized case, doesn't this send extra SL2VL mapping table ?
Couldn''t that be eliminated by checking for this in the new endport
routine (and skipping the remainder of that routine) ?

-- Hal

>
> 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
>
--
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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check
  2010-01-04 17:01   ` [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check Sasha Khapyorsky
  2010-01-05 11:10     ` [PATCH] opensm/osm_qos.c: split switch external and end ports setup Sasha Khapyorsky
@ 2010-01-07 13:38     ` Hal Rosenstock
       [not found]       ` <f0e08f231001070538q78453f91kd47c931087dfa735-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-07 13:38 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Mon, Jan 4, 2010 at 12:01 PM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
>
> The IBA 1.2.1 states (Vol.1, p.840, Table XX,

Table 146 PortInfo

> explanation (b)) that
> SL2VL mapping capability for switch external ports must be indicated on
> its port 0 PortInfo:CapMask.

That footnote was added at IBA 1.2.1.

> So it lets us to unify this capability check over all types of IB nodes.

Change appears to be for switches to always rely on this bit rather
than only when VLCap is 1. I wonder if there are any switches with
VLCap > 1 that don't set the IsSLMappingSupported CapabilityMask bit.
There shouldn't be (at least if they are IBA 1.2.1 compliant) but are
you sure about this ?

-- Hal

> Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
> ---
>  opensm/opensm/osm_qos.c |   22 +++++++---------------
>  1 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> index 08f9a60..afeaa11 100644
> --- a/opensm/opensm/osm_qos.c
> +++ b/opensm/opensm/osm_qos.c
> @@ -194,23 +194,15 @@ static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
>  {
>        ib_api_status_t status;
>        uint8_t i, num_ports;
> -       osm_physp_t *p_physp;
> +       ib_port_info_t *pi = &p_port->p_physp->port_info;
> +
> +       if (!(pi->capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> +               return IB_SUCCESS;
>
> -       if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH) {
> -               if (ib_port_info_get_vl_cap(&p->port_info) == 1) {
> -                       /* Check port 0's capability mask */
> -                       p_physp = p_port->p_physp;
> -                       if (!
> -                           (p_physp->port_info.
> -                            capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> -                               return IB_SUCCESS;
> -               }
> +       if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH)
>                num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
> -       } else {
> -               if (!(p->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> -                       return IB_SUCCESS;
> +       else
>                num_ports = 1;
> -       }
>
>        for (i = 0; i < num_ports; i++) {
>                status = sl2vl_update_table(sm, p, i, port_num, force_update,
> --
> 1.6.6
>
> --
> 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
>
--
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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: split switch external and end ports setup
  2010-01-05 11:10     ` [PATCH] opensm/osm_qos.c: split switch external and end ports setup Sasha Khapyorsky
@ 2010-01-07 13:40       ` Hal Rosenstock
       [not found]         ` <f0e08f231001070540y26b69cb3v82dddc1d7b7134b9-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-07 13:40 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Eli Dorfman

On Tue, Jan 5, 2010 at 6:10 AM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
>
> This splits QoS related port parameters setup over switch external ports
> and end ports. Such separation leaves us with simpler code and saves
> some repeated flows in case of switch external ports (actually required
> per switch and not per port).

Not sure what you mean by required in this statement. What are you
referring to as required per switch rather than per port ?

> Another advantages: Optimized Sl2VL Mapping procedure can be implemented
> easier using this model. A low level port QoS related parameters setup
> infrastructure is ready for supporting per port QoS related configuration
> (which hopefully will be implemented some days).
>
> Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
> ---
>  opensm/opensm/osm_qos.c |  137 +++++++++++++++++++++--------------------------
>  1 files changed, 61 insertions(+), 76 deletions(-)
>
> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> index afeaa11..f54e995 100644
> --- a/opensm/opensm/osm_qos.c
> +++ b/opensm/opensm/osm_qos.c
> @@ -77,6 +77,7 @@ static ib_api_status_t vlarb_update_table_block(osm_sm_t * sm,
>        osm_madw_context_t context;
>        uint32_t attr_mod;
>        unsigned vl_mask, i;
> +       ib_api_status_t status;
>
>        vl_mask = (1 << (ib_port_info_get_op_vls(&p->port_info) - 1)) - 1;
>
> @@ -96,10 +97,17 @@ static ib_api_status_t vlarb_update_table_block(osm_sm_t * sm,
>        context.vla_context.set_method = TRUE;
>        attr_mod = ((block_num + 1) << 16) | port_num;
>
> -       return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> -                          (uint8_t *) & block, sizeof(block),
> -                          IB_MAD_ATTR_VL_ARBITRATION, cl_hton32(attr_mod),
> -                          CL_DISP_MSGID_NONE, &context);
> +       status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> +                            (uint8_t *) & block, sizeof(block),
> +                            IB_MAD_ATTR_VL_ARBITRATION, cl_hton32(attr_mod),
> +                            CL_DISP_MSGID_NONE, &context);
> +       if (status != IB_SUCCESS)
> +               OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 6202 : "
> +                       "failed to update VLArbitration tables "
> +                       "for port %" PRIx64 " block %u\n",
> +                       cl_ntoh64(p->port_guid), block_num);
> +
> +       return status;
>  }
>
>  static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
> @@ -157,6 +165,7 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>        ib_slvl_table_t tbl, *p_tbl;
>        osm_node_t *p_node = osm_physp_get_node_ptr(p);
>        uint32_t attr_mod;
> +       ib_api_status_t status;
>        unsigned vl_mask;
>        uint8_t vl1, vl2;
>        int i;
> @@ -181,70 +190,65 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>        context.slvl_context.port_guid = osm_physp_get_port_guid(p);
>        context.slvl_context.set_method = TRUE;
>        attr_mod = in_port << 8 | out_port;
> -       return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> -                          (uint8_t *) & tbl, sizeof(tbl),
> -                          IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
> -                          CL_DISP_MSGID_NONE, &context);
> +       status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> +                            (uint8_t *) & tbl, sizeof(tbl),
> +                            IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
> +                            CL_DISP_MSGID_NONE, &context);
> +       if (status != IB_SUCCESS)
> +               OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 6203 : "
> +                       "failed to update SL2VLMapping tables "
> +                       "for port %" PRIx64 "i, attr_mod 0x%x\n",
> +                       cl_ntoh64(p->port_guid), attr_mod);
> +       return status;
>  }
>
> -static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
> -                                   osm_physp_t * p, uint8_t port_num,
> -                                   unsigned force_update,
> -                                   const struct qos_config *qcfg)
> +static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
> +                             const struct qos_config *qcfg)
>  {
> -       ib_api_status_t status;
> -       uint8_t i, num_ports;
> -       ib_port_info_t *pi = &p_port->p_physp->port_info;
> -
> -       if (!(pi->capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> -               return IB_SUCCESS;
> +       osm_physp_t *p0, *p;
> +       unsigned force_update;
> +       unsigned num_ports = osm_node_get_num_physp(node);
> +       int ret = 0;
> +       unsigned i, j;
>
> -       if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH)
> -               num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
> -       else
> -               num_ports = 1;
> +       for (i = 1; i < num_ports; i++) {
> +               p = osm_node_get_physp_ptr(node, i);
> +               force_update = p->need_update || sm->p_subn->need_update;
> +               p->vl_high_limit = qcfg->vl_high_limit;
> +               if (vlarb_update(sm, p, p->port_num, force_update, qcfg))
> +                       ret = -1;
> +       }
>
> -       for (i = 0; i < num_ports; i++) {
> -               status = sl2vl_update_table(sm, p, i, port_num, force_update,
> -                                           &qcfg->sl2vl);
> -               if (status != IB_SUCCESS)
> -                       return status;
> +       p0 = osm_node_get_physp_ptr(node, 0);
> +       if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> +               return ret;

Same comment on this as previous osm_qos patch comment on merging the
SL2VL mapping capability check.

-- Hal

> +
> +       for (i = 1; i < num_ports; i++) {
> +               p = osm_node_get_physp_ptr(node, i);
> +               force_update = p->need_update || sm->p_subn->need_update;
> +               for (j = 0; j < num_ports; j++)
> +                       if (sl2vl_update_table(sm, p, i, j, force_update,
> +                                              &qcfg->sl2vl))
> +                               ret = -1;
>        }
>
> -       return IB_SUCCESS;
> +       return ret;
>  }
>
> -static int qos_physp_setup(osm_log_t * p_log, osm_sm_t * sm,
> -                          osm_port_t * p_port, osm_physp_t * p,
> -                          uint8_t port_num, unsigned force_update,
> -                          const struct qos_config *qcfg)
> +static int qos_endport_setup(osm_sm_t * sm, osm_physp_t * p,
> +                            const struct qos_config *qcfg)
>  {
> -       ib_api_status_t status;
> +       unsigned force_update = p->need_update || sm->p_subn->need_update;
>
> -       /* OpVLs should be ok at this moment - just use it */
> -
> -       /* setup VL high limit on the physp later to be updated by link mgr */
>        p->vl_high_limit = qcfg->vl_high_limit;
> -
> -       /* setup VLArbitration */
> -       status = vlarb_update(sm, p, port_num, force_update, qcfg);
> -       if (status != IB_SUCCESS) {
> -               OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6202 : "
> -                       "failed to update VLArbitration tables "
> -                       "for port %" PRIx64 " #%d\n",
> -                       cl_ntoh64(p->port_guid), port_num);
> +       if (vlarb_update(sm, p, 0, force_update, qcfg))
>                return -1;
> -       }
>
> -       /* setup SL2VL tables */
> -       status = sl2vl_update(sm, p_port, p, port_num, force_update, qcfg);
> -       if (status != IB_SUCCESS) {
> -               OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 6203 : "
> -                       "failed to update SL2VLMapping tables "
> -                       "for port %" PRIx64 " #%d\n",
> -                       cl_ntoh64(p->port_guid), port_num);
> +       if (!(p->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> +               return 0;
> +
> +       if (sl2vl_update_table(sm, p, 0, 0, force_update, &qcfg->sl2vl))
>                return -1;
> -       }
>
>        return 0;
>  }
> @@ -256,12 +260,8 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>        cl_qmap_t *p_tbl;
>        cl_map_item_t *p_next;
>        osm_port_t *p_port;
> -       uint32_t num_physp;
> -       osm_physp_t *p_physp;
>        osm_node_t *p_node;
> -       unsigned force_update;
>        int ret = 0;
> -       uint8_t i;
>
>        if (!p_osm->subn.opt.qos)
>                return 0;
> @@ -290,18 +290,9 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>
>                p_node = p_port->p_node;
>                if (p_node->sw) {
> -                       num_physp = osm_node_get_num_physp(p_node);
> -                       for (i = 1; i < num_physp; i++) {
> -                               p_physp = osm_node_get_physp_ptr(p_node, i);
> -                               if (!p_physp)
> -                                       continue;
> -                               force_update = p_physp->need_update ||
> -                                   p_osm->subn.need_update;
> -                               if (qos_physp_setup(&p_osm->log, &p_osm->sm,
> -                                                   p_port, p_physp, i,
> -                                                   force_update, &swe_config))
> -                                       ret = -1;
> -                       }
> +                       if (qos_extports_setup(&p_osm->sm, p_node, &swe_config))
> +                               ret = -1;
> +
>                        /* skip base port 0 */
>                        if (!ib_switch_info_is_enhanced_port0
>                            (&p_node->sw->switch_info))
> @@ -313,13 +304,7 @@ int osm_qos_setup(osm_opensm_t * p_osm)
>                else
>                        cfg = &ca_config;
>
> -               p_physp = p_port->p_physp;
> -               if (!p_physp)
> -                       continue;
> -
> -               force_update = p_physp->need_update || p_osm->subn.need_update;
> -               if (qos_physp_setup(&p_osm->log, &p_osm->sm, p_port, p_physp,
> -                                   0, force_update, cfg))
> +               if (qos_endport_setup(&p_osm->sm, p_port->p_physp, cfg))
>                        ret = -1;
>        }
>
> --
> 1.6.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check
       [not found]       ` <f0e08f231001070538q78453f91kd47c931087dfa735-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-12 10:35         ` Sasha Khapyorsky
  2010-01-13 20:13           ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-12 10:35 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 08:38 Thu 07 Jan     , Hal Rosenstock wrote:
> 
> Change appears to be for switches to always rely on this bit rather
> than only when VLCap is 1. I wonder if there are any switches with
> VLCap > 1 that don't set the IsSLMappingSupported CapabilityMask bit.
> There shouldn't be (at least if they are IBA 1.2.1 compliant) but are
> you sure about this ?

I'm not sure about this, but think that probability of using such
hypothetical old switches for any sort of QoS is very low. And anyway it
doesn't look for me that we have any stronger SL2VL mapping capability
indication - 'VLCap > 1' by itself doesn't do this too, right?

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: split switch external and end ports setup
       [not found]         ` <f0e08f231001070540y26b69cb3v82dddc1d7b7134b9-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-12 10:43           ` Sasha Khapyorsky
  2010-01-13 20:15             ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-12 10:43 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Eli Dorfman

On 08:40 Thu 07 Jan     , Hal Rosenstock wrote:
> On Tue, Jan 5, 2010 at 6:10 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> >
> > This splits QoS related port parameters setup over switch external ports
> > and end ports. Such separation leaves us with simpler code and saves
> > some repeated flows in case of switch external ports (actually required
> > per switch and not per port).
> 
> Not sure what you mean by required in this statement. What are you
> referring to as required per switch rather than per port ?

Duplicated per switch external port setup detection of common (per
switch) data - such as: number of ports, Sl2VL mapping capability
p0->port_info.capability_mask.

> 
> > Another advantages: Optimized Sl2VL Mapping procedure can be implemented
> > easier using this model. A low level port QoS related parameters setup
> > infrastructure is ready for supporting per port QoS related configuration
> > (which hopefully will be implemented some days).
> >
> > Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
> > ---
> >  opensm/opensm/osm_qos.c |  137 +++++++++++++++++++++--------------------------
> >  1 files changed, 61 insertions(+), 76 deletions(-)
> >
> > diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> > index afeaa11..f54e995 100644
> > --- a/opensm/opensm/osm_qos.c
> > +++ b/opensm/opensm/osm_qos.c
> > @@ -77,6 +77,7 @@ static ib_api_status_t vlarb_update_table_block(osm_sm_t * sm,
> >        osm_madw_context_t context;
> >        uint32_t attr_mod;
> >        unsigned vl_mask, i;
> > +       ib_api_status_t status;
> >
> >        vl_mask = (1 << (ib_port_info_get_op_vls(&p->port_info) - 1)) - 1;
> >
> > @@ -96,10 +97,17 @@ static ib_api_status_t vlarb_update_table_block(osm_sm_t * sm,
> >        context.vla_context.set_method = TRUE;
> >        attr_mod = ((block_num + 1) << 16) | port_num;
> >
> > -       return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> > -                          (uint8_t *) & block, sizeof(block),
> > -                          IB_MAD_ATTR_VL_ARBITRATION, cl_hton32(attr_mod),
> > -                          CL_DISP_MSGID_NONE, &context);
> > +       status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> > +                            (uint8_t *) & block, sizeof(block),
> > +                            IB_MAD_ATTR_VL_ARBITRATION, cl_hton32(attr_mod),
> > +                            CL_DISP_MSGID_NONE, &context);
> > +       if (status != IB_SUCCESS)
> > +               OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 6202 : "
> > +                       "failed to update VLArbitration tables "
> > +                       "for port %" PRIx64 " block %u\n",
> > +                       cl_ntoh64(p->port_guid), block_num);
> > +
> > +       return status;
> >  }
> >
> >  static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
> > @@ -157,6 +165,7 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
> >        ib_slvl_table_t tbl, *p_tbl;
> >        osm_node_t *p_node = osm_physp_get_node_ptr(p);
> >        uint32_t attr_mod;
> > +       ib_api_status_t status;
> >        unsigned vl_mask;
> >        uint8_t vl1, vl2;
> >        int i;
> > @@ -181,70 +190,65 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
> >        context.slvl_context.port_guid = osm_physp_get_port_guid(p);
> >        context.slvl_context.set_method = TRUE;
> >        attr_mod = in_port << 8 | out_port;
> > -       return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> > -                          (uint8_t *) & tbl, sizeof(tbl),
> > -                          IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
> > -                          CL_DISP_MSGID_NONE, &context);
> > +       status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> > +                            (uint8_t *) & tbl, sizeof(tbl),
> > +                            IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
> > +                            CL_DISP_MSGID_NONE, &context);
> > +       if (status != IB_SUCCESS)
> > +               OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 6203 : "
> > +                       "failed to update SL2VLMapping tables "
> > +                       "for port %" PRIx64 "i, attr_mod 0x%x\n",
> > +                       cl_ntoh64(p->port_guid), attr_mod);
> > +       return status;
> >  }
> >
> > -static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
> > -                                   osm_physp_t * p, uint8_t port_num,
> > -                                   unsigned force_update,
> > -                                   const struct qos_config *qcfg)
> > +static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
> > +                             const struct qos_config *qcfg)
> >  {
> > -       ib_api_status_t status;
> > -       uint8_t i, num_ports;
> > -       ib_port_info_t *pi = &p_port->p_physp->port_info;
> > -
> > -       if (!(pi->capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> > -               return IB_SUCCESS;
> > +       osm_physp_t *p0, *p;
> > +       unsigned force_update;
> > +       unsigned num_ports = osm_node_get_num_physp(node);
> > +       int ret = 0;
> > +       unsigned i, j;
> >
> > -       if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH)
> > -               num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
> > -       else
> > -               num_ports = 1;
> > +       for (i = 1; i < num_ports; i++) {
> > +               p = osm_node_get_physp_ptr(node, i);
> > +               force_update = p->need_update || sm->p_subn->need_update;
> > +               p->vl_high_limit = qcfg->vl_high_limit;
> > +               if (vlarb_update(sm, p, p->port_num, force_update, qcfg))
> > +                       ret = -1;
> > +       }
> >
> > -       for (i = 0; i < num_ports; i++) {
> > -               status = sl2vl_update_table(sm, p, i, port_num, force_update,
> > -                                           &qcfg->sl2vl);
> > -               if (status != IB_SUCCESS)
> > -                       return status;
> > +       p0 = osm_node_get_physp_ptr(node, 0);
> > +       if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> > +               return ret;
> 
> Same comment on this as previous osm_qos patch comment on merging the
> SL2VL mapping capability check.

I answered there.

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
       [not found]         ` <f0e08f231001070535l382f1d54t7f6290607e52b196-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-12 10:55           ` Sasha Khapyorsky
  2010-01-13 20:14             ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-12 10:55 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 08:35 Thu 07 Jan     , Hal Rosenstock wrote:
> >
> > diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> > index f54e995..6bbbfa2 100644
> > --- a/opensm/opensm/osm_qos.c
> > +++ b/opensm/opensm/osm_qos.c
> > @@ -1,5 +1,6 @@
> >  /*
> >  * Copyright (c) 2006-2009 Voltaire, Inc. All rights reserved.
> > + * Copyright (c) 2009 HNR Consulting. All rights reserved.
> >  *
> >  * This software is available to you under a choice of one of two
> >  * licenses.  You may choose to be licensed under the terms of the GNU
> > @@ -157,14 +158,13 @@ static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
> >  }
> >
> >  static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
> > -                                         uint8_t in_port, uint8_t out_port,
> > +                                         uint8_t in_port, uint32_t attr_mod,
> >                                          unsigned force_update,
> >                                          const ib_slvl_table_t * sl2vl_table)
> >  {
> >        osm_madw_context_t context;
> >        ib_slvl_table_t tbl, *p_tbl;
> >        osm_node_t *p_node = osm_physp_get_node_ptr(p);
> > -       uint32_t attr_mod;
> >        ib_api_status_t status;
> >        unsigned vl_mask;
> >        uint8_t vl1, vl2;
> > @@ -189,7 +189,6 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
> >        context.slvl_context.node_guid = osm_node_get_node_guid(p_node);
> >        context.slvl_context.port_guid = osm_physp_get_port_guid(p);
> >        context.slvl_context.set_method = TRUE;
> > -       attr_mod = in_port << 8 | out_port;
> >        status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
> >                             (uint8_t *) & tbl, sizeof(tbl),
> >                             IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
> > @@ -223,12 +222,20 @@ static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
> >        if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> >                return ret;
> >
> > +       if (ib_switch_info_get_opt_sl2vlmapping(&node->sw->switch_info) &&
> > +           sm->p_subn->opt.use_optimized_slvl) {
> > +               p = osm_node_get_physp_ptr(node, 1);
> > +               force_update = p->need_update || sm->p_subn->need_update;
> > +               return sl2vl_update_table(sm, p, 1, 0x30000, force_update,
> > +                                         &qcfg->sl2vl);
> > +       }
> > +
> >        for (i = 1; i < num_ports; i++) {
> >                p = osm_node_get_physp_ptr(node, i);
> >                force_update = p->need_update || sm->p_subn->need_update;
> >                for (j = 0; j < num_ports; j++)
> > -                       if (sl2vl_update_table(sm, p, i, j, force_update,
> > -                                              &qcfg->sl2vl))
> > +                       if (sl2vl_update_table(sm, p, i, i << 8 | j,
> > +                                              force_update, &qcfg->sl2vl))
> >                                ret = -1;
> >        }
> >
> >
> > , does it look fine for you?
> 
> In the optimized case, doesn't this send extra SL2VL mapping table ?

I don't see how, could you elaborate?

Sasha

> Couldn''t that be eliminated by checking for this in the new endport
> routine (and skipping the remainder of that routine) ?
> 
> -- Hal
> 
> >
> > 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
> >
> 
--
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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check
  2010-01-12 10:35         ` Sasha Khapyorsky
@ 2010-01-13 20:13           ` Hal Rosenstock
       [not found]             ` <f0e08f231001131213r4a743d6boc1b07e9fadc773ac-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-13 20:13 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Tue, Jan 12, 2010 at 5:35 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 08:38 Thu 07 Jan     , Hal Rosenstock wrote:
>>
>> Change appears to be for switches to always rely on this bit rather
>> than only when VLCap is 1. I wonder if there are any switches with
>> VLCap > 1 that don't set the IsSLMappingSupported CapabilityMask bit.
>> There shouldn't be (at least if they are IBA 1.2.1 compliant) but are
>> you sure about this ?
>
> I'm not sure about this, but think that probability of using such
> hypothetical old switches for any sort of QoS is very low.

Low but not zero... I'm also not sure it's just old switches...
I've seen many rejections (status 7) since you made this change.

> And anyway it
> doesn't look for me that we have any stronger SL2VL mapping capability
> indication - 'VLCap > 1' by itself doesn't do this too, right?

It does. There's a requirement that SL2VL mapping is required when VLCap > 1.

-- Hal

> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
  2010-01-12 10:55           ` Sasha Khapyorsky
@ 2010-01-13 20:14             ` Hal Rosenstock
       [not found]               ` <f0e08f231001131214w790b1f34s46cbbda8b6454ac3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-13 20:14 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Tue, Jan 12, 2010 at 5:55 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 08:35 Thu 07 Jan     , Hal Rosenstock wrote:
>> >
>> > diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
>> > index f54e995..6bbbfa2 100644
>> > --- a/opensm/opensm/osm_qos.c
>> > +++ b/opensm/opensm/osm_qos.c
>> > @@ -1,5 +1,6 @@
>> >  /*
>> >  * Copyright (c) 2006-2009 Voltaire, Inc. All rights reserved.
>> > + * Copyright (c) 2009 HNR Consulting. All rights reserved.
>> >  *
>> >  * This software is available to you under a choice of one of two
>> >  * licenses.  You may choose to be licensed under the terms of the GNU
>> > @@ -157,14 +158,13 @@ static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
>> >  }
>> >
>> >  static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>> > -                                         uint8_t in_port, uint8_t out_port,
>> > +                                         uint8_t in_port, uint32_t attr_mod,
>> >                                          unsigned force_update,
>> >                                          const ib_slvl_table_t * sl2vl_table)
>> >  {
>> >        osm_madw_context_t context;
>> >        ib_slvl_table_t tbl, *p_tbl;
>> >        osm_node_t *p_node = osm_physp_get_node_ptr(p);
>> > -       uint32_t attr_mod;
>> >        ib_api_status_t status;
>> >        unsigned vl_mask;
>> >        uint8_t vl1, vl2;
>> > @@ -189,7 +189,6 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>> >        context.slvl_context.node_guid = osm_node_get_node_guid(p_node);
>> >        context.slvl_context.port_guid = osm_physp_get_port_guid(p);
>> >        context.slvl_context.set_method = TRUE;
>> > -       attr_mod = in_port << 8 | out_port;
>> >        status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
>> >                             (uint8_t *) & tbl, sizeof(tbl),
>> >                             IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
>> > @@ -223,12 +222,20 @@ static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
>> >        if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>> >                return ret;
>> >
>> > +       if (ib_switch_info_get_opt_sl2vlmapping(&node->sw->switch_info) &&
>> > +           sm->p_subn->opt.use_optimized_slvl) {
>> > +               p = osm_node_get_physp_ptr(node, 1);
>> > +               force_update = p->need_update || sm->p_subn->need_update;
>> > +               return sl2vl_update_table(sm, p, 1, 0x30000, force_update,
>> > +                                         &qcfg->sl2vl);
>> > +       }
>> > +
>> >        for (i = 1; i < num_ports; i++) {
>> >                p = osm_node_get_physp_ptr(node, i);
>> >                force_update = p->need_update || sm->p_subn->need_update;
>> >                for (j = 0; j < num_ports; j++)
>> > -                       if (sl2vl_update_table(sm, p, i, j, force_update,
>> > -                                              &qcfg->sl2vl))
>> > +                       if (sl2vl_update_table(sm, p, i, i << 8 | j,
>> > +                                              force_update, &qcfg->sl2vl))
>> >                                ret = -1;
>> >        }
>> >
>> >
>> > , does it look fine for you?
>>
>> In the optimized case, doesn't this send extra SL2VL mapping table ?
>
> I don't see how, could you elaborate?

Doesn't this send a table for the endport even when optimized ?

-- Hal

>
> Sasha
>
>> Couldn''t that be eliminated by checking for this in the new endport
>> routine (and skipping the remainder of that routine) ?
>>
>> -- Hal
>>
>> >
>> > 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
>> >
>>
>
--
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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: split switch external and end ports setup
  2010-01-12 10:43           ` Sasha Khapyorsky
@ 2010-01-13 20:15             ` Hal Rosenstock
  0 siblings, 0 replies; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-13 20:15 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Eli Dorfman

On Tue, Jan 12, 2010 at 5:43 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 08:40 Thu 07 Jan     , Hal Rosenstock wrote:
>> On Tue, Jan 5, 2010 at 6:10 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
>> >
>> > This splits QoS related port parameters setup over switch external ports
>> > and end ports. Such separation leaves us with simpler code and saves
>> > some repeated flows in case of switch external ports (actually required
>> > per switch and not per port).
>>
>> Not sure what you mean by required in this statement. What are you
>> referring to as required per switch rather than per port ?
>
> Duplicated per switch external port setup detection of common (per
> switch) data - such as: number of ports, Sl2VL mapping capability
> p0->port_info.capability_mask.

It was your use of the word 'required' which was confusing to me. It's
clear now what you mean.

-- Hal

>
>>
>> > Another advantages: Optimized Sl2VL Mapping procedure can be implemented
>> > easier using this model. A low level port QoS related parameters setup
>> > infrastructure is ready for supporting per port QoS related configuration
>> > (which hopefully will be implemented some days).
>> >
>> > Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
>> > ---
>> >  opensm/opensm/osm_qos.c |  137 +++++++++++++++++++++--------------------------
>> >  1 files changed, 61 insertions(+), 76 deletions(-)
>> >
>> > diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
>> > index afeaa11..f54e995 100644
>> > --- a/opensm/opensm/osm_qos.c
>> > +++ b/opensm/opensm/osm_qos.c
>> > @@ -77,6 +77,7 @@ static ib_api_status_t vlarb_update_table_block(osm_sm_t * sm,
>> >        osm_madw_context_t context;
>> >        uint32_t attr_mod;
>> >        unsigned vl_mask, i;
>> > +       ib_api_status_t status;
>> >
>> >        vl_mask = (1 << (ib_port_info_get_op_vls(&p->port_info) - 1)) - 1;
>> >
>> > @@ -96,10 +97,17 @@ static ib_api_status_t vlarb_update_table_block(osm_sm_t * sm,
>> >        context.vla_context.set_method = TRUE;
>> >        attr_mod = ((block_num + 1) << 16) | port_num;
>> >
>> > -       return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
>> > -                          (uint8_t *) & block, sizeof(block),
>> > -                          IB_MAD_ATTR_VL_ARBITRATION, cl_hton32(attr_mod),
>> > -                          CL_DISP_MSGID_NONE, &context);
>> > +       status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
>> > +                            (uint8_t *) & block, sizeof(block),
>> > +                            IB_MAD_ATTR_VL_ARBITRATION, cl_hton32(attr_mod),
>> > +                            CL_DISP_MSGID_NONE, &context);
>> > +       if (status != IB_SUCCESS)
>> > +               OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 6202 : "
>> > +                       "failed to update VLArbitration tables "
>> > +                       "for port %" PRIx64 " block %u\n",
>> > +                       cl_ntoh64(p->port_guid), block_num);
>> > +
>> > +       return status;
>> >  }
>> >
>> >  static ib_api_status_t vlarb_update(osm_sm_t * sm, osm_physp_t * p,
>> > @@ -157,6 +165,7 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>> >        ib_slvl_table_t tbl, *p_tbl;
>> >        osm_node_t *p_node = osm_physp_get_node_ptr(p);
>> >        uint32_t attr_mod;
>> > +       ib_api_status_t status;
>> >        unsigned vl_mask;
>> >        uint8_t vl1, vl2;
>> >        int i;
>> > @@ -181,70 +190,65 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, osm_physp_t * p,
>> >        context.slvl_context.port_guid = osm_physp_get_port_guid(p);
>> >        context.slvl_context.set_method = TRUE;
>> >        attr_mod = in_port << 8 | out_port;
>> > -       return osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
>> > -                          (uint8_t *) & tbl, sizeof(tbl),
>> > -                          IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
>> > -                          CL_DISP_MSGID_NONE, &context);
>> > +       status = osm_req_set(sm, osm_physp_get_dr_path_ptr(p),
>> > +                            (uint8_t *) & tbl, sizeof(tbl),
>> > +                            IB_MAD_ATTR_SLVL_TABLE, cl_hton32(attr_mod),
>> > +                            CL_DISP_MSGID_NONE, &context);
>> > +       if (status != IB_SUCCESS)
>> > +               OSM_LOG(sm->p_log, OSM_LOG_ERROR, "ERR 6203 : "
>> > +                       "failed to update SL2VLMapping tables "
>> > +                       "for port %" PRIx64 "i, attr_mod 0x%x\n",
>> > +                       cl_ntoh64(p->port_guid), attr_mod);
>> > +       return status;
>> >  }
>> >
>> > -static ib_api_status_t sl2vl_update(osm_sm_t * sm, osm_port_t * p_port,
>> > -                                   osm_physp_t * p, uint8_t port_num,
>> > -                                   unsigned force_update,
>> > -                                   const struct qos_config *qcfg)
>> > +static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
>> > +                             const struct qos_config *qcfg)
>> >  {
>> > -       ib_api_status_t status;
>> > -       uint8_t i, num_ports;
>> > -       ib_port_info_t *pi = &p_port->p_physp->port_info;
>> > -
>> > -       if (!(pi->capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>> > -               return IB_SUCCESS;
>> > +       osm_physp_t *p0, *p;
>> > +       unsigned force_update;
>> > +       unsigned num_ports = osm_node_get_num_physp(node);
>> > +       int ret = 0;
>> > +       unsigned i, j;
>> >
>> > -       if (osm_node_get_type(osm_physp_get_node_ptr(p)) == IB_NODE_TYPE_SWITCH)
>> > -               num_ports = osm_node_get_num_physp(osm_physp_get_node_ptr(p));
>> > -       else
>> > -               num_ports = 1;
>> > +       for (i = 1; i < num_ports; i++) {
>> > +               p = osm_node_get_physp_ptr(node, i);
>> > +               force_update = p->need_update || sm->p_subn->need_update;
>> > +               p->vl_high_limit = qcfg->vl_high_limit;
>> > +               if (vlarb_update(sm, p, p->port_num, force_update, qcfg))
>> > +                       ret = -1;
>> > +       }
>> >
>> > -       for (i = 0; i < num_ports; i++) {
>> > -               status = sl2vl_update_table(sm, p, i, port_num, force_update,
>> > -                                           &qcfg->sl2vl);
>> > -               if (status != IB_SUCCESS)
>> > -                       return status;
>> > +       p0 = osm_node_get_physp_ptr(node, 0);
>> > +       if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>> > +               return ret;
>>
>> Same comment on this as previous osm_qos patch comment on merging the
>> SL2VL mapping capability check.
>
> I answered there.
>
> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check
       [not found]             ` <f0e08f231001131213r4a743d6boc1b07e9fadc773ac-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-16 20:27               ` Sasha Khapyorsky
  2010-01-23 12:25                 ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-16 20:27 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 15:13 Wed 13 Jan     , Hal Rosenstock wrote:
> On Tue, Jan 12, 2010 at 5:35 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> > On 08:38 Thu 07 Jan     , Hal Rosenstock wrote:
> >>
> >> Change appears to be for switches to always rely on this bit rather
> >> than only when VLCap is 1. I wonder if there are any switches with
> >> VLCap > 1 that don't set the IsSLMappingSupported CapabilityMask bit.
> >> There shouldn't be (at least if they are IBA 1.2.1 compliant) but are
> >> you sure about this ?
> >
> > I'm not sure about this, but think that probability of using such
> > hypothetical old switches for any sort of QoS is very low.
> 
> Low but not zero... I'm also not sure it's just old switches...
> I've seen many rejections (status 7) since you made this change.

This is interesting. Do you have any details?

> > And anyway it
> > doesn't look for me that we have any stronger SL2VL mapping capability
> > indication - 'VLCap > 1' by itself doesn't do this too, right?
> 
> It does. There's a requirement that SL2VL mapping is required when VLCap > 1.

Correct, I found it in o7-4. Basically we can check both (now after
switch/endport separation in the code this should be easier), but I
would prefer to understand better an issue (if we have) first.

Also same o7-4 is applicable to CA and router ports. Do you know why
was VLCap > 1 condition ignored there?

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
       [not found]               ` <f0e08f231001131214w790b1f34s46cbbda8b6454ac3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-16 20:33                 ` Sasha Khapyorsky
  2010-01-23 12:26                   ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-16 20:33 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 15:14 Wed 13 Jan     , Hal Rosenstock wrote:
> >> > @@ -223,12 +222,20 @@ static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
> >> >        if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
> >> >                return ret;
> >> >
> >> > +       if (ib_switch_info_get_opt_sl2vlmapping(&node->sw->switch_info) &&
> >> > +           sm->p_subn->opt.use_optimized_slvl) {
> >> > +               p = osm_node_get_physp_ptr(node, 1);
> >> > +               force_update = p->need_update || sm->p_subn->need_update;
> >> > +               return sl2vl_update_table(sm, p, 1, 0x30000, force_update,
> >> > +                                         &qcfg->sl2vl);
> >> > +       }
> >> > +
> >> >        for (i = 1; i < num_ports; i++) {
> >> >                p = osm_node_get_physp_ptr(node, i);
> >> >                force_update = p->need_update || sm->p_subn->need_update;
> >> >                for (j = 0; j < num_ports; j++)
> >> > -                       if (sl2vl_update_table(sm, p, i, j, force_update,
> >> > -                                              &qcfg->sl2vl))
> >> > +                       if (sl2vl_update_table(sm, p, i, i << 8 | j,
> >> > +                                              force_update, &qcfg->sl2vl))
> >> >                                ret = -1;
> >> >        }
> >> >
> >> >
> >> > , does it look fine for you?
> >>
> >> In the optimized case, doesn't this send extra SL2VL mapping table ?
> >
> > I don't see how, could you elaborate?
> 
> Doesn't this send a table for the endport even when optimized ?

Yes, and how is this different from the original patch?

Remember, that there is a different configuration for switch's port 0
(needed or not - this is another question), so we must to take it into
account.

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check
  2010-01-16 20:27               ` Sasha Khapyorsky
@ 2010-01-23 12:25                 ` Hal Rosenstock
       [not found]                   ` <f0e08f231001230425j28ced479if608a75bb4216186-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-23 12:25 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Sat, Jan 16, 2010 at 3:27 PM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 15:13 Wed 13 Jan     , Hal Rosenstock wrote:
>> On Tue, Jan 12, 2010 at 5:35 AM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
>> > On 08:38 Thu 07 Jan     , Hal Rosenstock wrote:
>> >>
>> >> Change appears to be for switches to always rely on this bit rather
>> >> than only when VLCap is 1. I wonder if there are any switches with
>> >> VLCap > 1 that don't set the IsSLMappingSupported CapabilityMask bit.
>> >> There shouldn't be (at least if they are IBA 1.2.1 compliant) but are
>> >> you sure about this ?
>> >
>> > I'm not sure about this, but think that probability of using such
>> > hypothetical old switches for any sort of QoS is very low.
>>
>> Low but not zero... I'm also not sure it's just old switches...
>> I've seen many rejections (status 7) since you made this change.
>
> This is interesting. Do you have any details?

I've been unable to get test time for this as yet.

>> > And anyway it
>> > doesn't look for me that we have any stronger SL2VL mapping capability
>> > indication - 'VLCap > 1' by itself doesn't do this too, right?
>>
>> It does. There's a requirement that SL2VL mapping is required when VLCap > 1.
>
> Correct, I found it in o7-4. Basically we can check both (now after
> switch/endport separation in the code this should be easier), but I
> would prefer to understand better an issue (if we have) first.
>
> Also same o7-4 is applicable to CA and router ports.

I see no mention of CA and router ports in o7-4. What are you
referring to here ?

> Do you know why
> was VLCap > 1 condition ignored there?

Where is the "there" that you are referring to above ?

-- Hal

> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
  2010-01-16 20:33                 ` Sasha Khapyorsky
@ 2010-01-23 12:26                   ` Hal Rosenstock
       [not found]                     ` <f0e08f231001230426t2e48aa58pbcb2d3f1323c6e56-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-23 12:26 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Sat, Jan 16, 2010 at 3:33 PM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 15:14 Wed 13 Jan     , Hal Rosenstock wrote:
>> >> > @@ -223,12 +222,20 @@ static int qos_extports_setup(osm_sm_t * sm, osm_node_t *node,
>> >> >        if (!(p0->port_info.capability_mask & IB_PORT_CAP_HAS_SL_MAP))
>> >> >                return ret;
>> >> >
>> >> > +       if (ib_switch_info_get_opt_sl2vlmapping(&node->sw->switch_info) &&
>> >> > +           sm->p_subn->opt.use_optimized_slvl) {
>> >> > +               p = osm_node_get_physp_ptr(node, 1);
>> >> > +               force_update = p->need_update || sm->p_subn->need_update;
>> >> > +               return sl2vl_update_table(sm, p, 1, 0x30000, force_update,
>> >> > +                                         &qcfg->sl2vl);
>> >> > +       }
>> >> > +
>> >> >        for (i = 1; i < num_ports; i++) {
>> >> >                p = osm_node_get_physp_ptr(node, i);
>> >> >                force_update = p->need_update || sm->p_subn->need_update;
>> >> >                for (j = 0; j < num_ports; j++)
>> >> > -                       if (sl2vl_update_table(sm, p, i, j, force_update,
>> >> > -                                              &qcfg->sl2vl))
>> >> > +                       if (sl2vl_update_table(sm, p, i, i << 8 | j,
>> >> > +                                              force_update, &qcfg->sl2vl))
>> >> >                                ret = -1;
>> >> >        }
>> >> >
>> >> >
>> >> > , does it look fine for you?
>> >>
>> >> In the optimized case, doesn't this send extra SL2VL mapping table ?
>> >
>> > I don't see how, could you elaborate?
>>
>> Doesn't this send a table for the endport even when optimized ?
>
> Yes, and how is this different from the original patch?

The original patch didn't do this but missed accounting for the switch
port 0 configuration.

> Remember, that there is a different configuration for switch's port 0
> (needed or not - this is another question), so we must to take it into
> account.

Yes but is there a need for this if they are the same and wildcarding
is used ? That will save what seems to me to be an unneeded MAD
request/response pair per switch.

-- Hal

> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check
       [not found]                   ` <f0e08f231001230425j28ced479if608a75bb4216186-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-25 15:20                     ` Sasha Khapyorsky
  2010-01-25 20:19                       ` Hal Rosenstock
  0 siblings, 1 reply; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-25 15:20 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 07:25 Sat 23 Jan     , Hal Rosenstock wrote:
> >>
> >> It does. There's a requirement that SL2VL mapping is required when VLCap > 1.
> >
> > Correct, I found it in o7-4. Basically we can check both (now after
> > switch/endport separation in the code this should be easier), but I
> > would prefer to understand better an issue (if we have) first.
> >
> > Also same o7-4 is applicable to CA and router ports.
> 
> I see no mention of CA and router ports in o7-4.

This is the point. o7-4 is not limited by switch ports.

> What are you
> referring to here ?

This was the question, why such capability check should be (was)
ideologically different for CA ports? If 'VLCap > 1' is significant
capability indication for switch ports this should be the same
interpretation with CA and router ports.

> 
> > Do you know why
> > was VLCap > 1 condition ignored there?
> 
> Where is the "there" that you are referring to above ?

not switch ports.

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming
       [not found]                     ` <f0e08f231001230426t2e48aa58pbcb2d3f1323c6e56-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-01-25 17:41                       ` Sasha Khapyorsky
  0 siblings, 0 replies; 37+ messages in thread
From: Sasha Khapyorsky @ 2010-01-25 17:41 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 07:26 Sat 23 Jan     , Hal Rosenstock wrote:
> >
> > Yes, and how is this different from the original patch?
> 
> The original patch didn't do this but missed accounting for the switch
> port 0 configuration.

I cannot understand. What do you mean?

> > Remember, that there is a different configuration for switch's port 0
> > (needed or not - this is another question), so we must to take it into
> > account.
> 
> Yes but is there a need for this if they are the same and wildcarding
> is used ? That will save what seems to me to be an unneeded MAD
> request/response pair per switch.

This leads us to switch ports (0 and externals) QoS configuration
unification. Basically I have nothing against this, but this is not
actually related to this patch series. And should be done first as
completely separate change, then we can merge SL2VL setup too.

However, there is some related long time pended task - QoS policy and
QoS port parameters configurations merge. Keeping this in the mind I
would think that finally we will need ability to have "per-port" SL2VL
and VLArb configuration. We can take it into account even now.

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

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check
  2010-01-25 15:20                     ` Sasha Khapyorsky
@ 2010-01-25 20:19                       ` Hal Rosenstock
  0 siblings, 0 replies; 37+ messages in thread
From: Hal Rosenstock @ 2010-01-25 20:19 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Mon, Jan 25, 2010 at 10:20 AM, Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:
> On 07:25 Sat 23 Jan     , Hal Rosenstock wrote:
>> >>
>> >> It does. There's a requirement that SL2VL mapping is required when VLCap > 1.
>> >
>> > Correct, I found it in o7-4. Basically we can check both (now after
>> > switch/endport separation in the code this should be easier), but I
>> > would prefer to understand better an issue (if we have) first.
>> >
>> > Also same o7-4 is applicable to CA and router ports.
>>
>> I see no mention of CA and router ports in o7-4.
>
> This is the point. o7-4 is not limited by switch ports.
>
>> What are you
>> referring to here ?
>
> This was the question, why such capability check should be (was)
> ideologically different for CA ports? If 'VLCap > 1' is significant
> capability indication for switch ports this should be the same
> interpretation with CA and router ports.

The difference is due to capability mask being valid for CA and router
ports and only valid for switch port 0. Later on (at IBA 1.2.1), we
came back and said for switch external ports they were the same as
switch port 0 which was how footnote (b) came to be added but I'm not
sure about relying on the conformance to this.

-- Hal

>
>>
>> > Do you know why
>> > was VLCap > 1 condition ignored there?
>>
>> Where is the "there" that you are referring to above ?
>
> not switch ports.
>
> 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

^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2010-01-25 20:19 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-01 19:41 [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Hal Rosenstock
     [not found] ` <20091201194110.GA26753-Wuw85uim5zDR7s880joybQ@public.gmane.org>
2009-12-29 15:05   ` [PATCH] opensm/osm_slvl_map_rcv.c: fix port parsing on BE machine Sasha Khapyorsky
2009-12-29 15:07     ` [PATCH] opensm/osm_slvl_map_rcv.c: verify port number values received from network Sasha Khapyorsky
2009-12-30 16:09       ` Hal Rosenstock
     [not found]         ` <f0e08f230912300809l7c794daat9490c64a2cf63525-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-12-30 16:27           ` Sasha Khapyorsky
2009-12-30 16:56             ` Hal Rosenstock
     [not found]               ` <f0e08f230912300856v188d5900u9d82c94f5f9ae4a6-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-12-30 19:43                 ` Sasha Khapyorsky
2009-12-31 14:48                   ` Hal Rosenstock
     [not found]                     ` <f0e08f230912310648m4b03b9bdq92394a61866f0e26-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-03 17:02                       ` Sasha Khapyorsky
2010-01-04 19:12                         ` Hal Rosenstock
     [not found]                           ` <f0e08f231001041112k4cb7742bwb5ae7cade8704082-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-04 20:03                             ` Sasha Khapyorsky
2010-01-04 19:31             ` Hal Rosenstock
2009-12-29 15:06   ` [PATCH] opensm/osm_slvl_map_rcv.c: fix mutex double release bug Sasha Khapyorsky
2009-12-29 16:29   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
2009-12-30 16:08     ` Hal Rosenstock
2010-01-04 17:01   ` [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check Sasha Khapyorsky
2010-01-05 11:10     ` [PATCH] opensm/osm_qos.c: split switch external and end ports setup Sasha Khapyorsky
2010-01-07 13:40       ` Hal Rosenstock
     [not found]         ` <f0e08f231001070540y26b69cb3v82dddc1d7b7134b9-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-12 10:43           ` Sasha Khapyorsky
2010-01-13 20:15             ` Hal Rosenstock
2010-01-07 13:38     ` [PATCH] opensm/osm_qos.c: merge SL2VL mapping capability check Hal Rosenstock
     [not found]       ` <f0e08f231001070538q78453f91kd47c931087dfa735-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-12 10:35         ` Sasha Khapyorsky
2010-01-13 20:13           ` Hal Rosenstock
     [not found]             ` <f0e08f231001131213r4a743d6boc1b07e9fadc773ac-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-16 20:27               ` Sasha Khapyorsky
2010-01-23 12:25                 ` Hal Rosenstock
     [not found]                   ` <f0e08f231001230425j28ced479if608a75bb4216186-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-25 15:20                     ` Sasha Khapyorsky
2010-01-25 20:19                       ` Hal Rosenstock
2010-01-05 11:18   ` [PATCHv3] opensm: Add support for optimized SLtoVLMappingTable programming Sasha Khapyorsky
2010-01-05 11:23     ` Sasha Khapyorsky
2010-01-07 13:35       ` Hal Rosenstock
     [not found]         ` <f0e08f231001070535l382f1d54t7f6290607e52b196-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-12 10:55           ` Sasha Khapyorsky
2010-01-13 20:14             ` Hal Rosenstock
     [not found]               ` <f0e08f231001131214w790b1f34s46cbbda8b6454ac3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-16 20:33                 ` Sasha Khapyorsky
2010-01-23 12:26                   ` Hal Rosenstock
     [not found]                     ` <f0e08f231001230426t2e48aa58pbcb2d3f1323c6e56-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-25 17:41                       ` Sasha Khapyorsky
2010-01-07 13:34     ` Hal Rosenstock
2010-01-06 19:13   ` Sasha Khapyorsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox