Hi Hal, On Mon, Aug 23, 2010 at 5:25 PM, Hal Rosenstock <[email protected]> wrote: > On Wed, Jul 28, 2010 at 12:26 PM, Eli Dorfman (Voltaire) > <[email protected]> wrote: >> >> Subject: [PATCH] Fix sl2vl configuration >> >> For non-optimized sl2vl configuration in and out ports were reversed. > > Are you sure these are reversed ? Any idea which commit introduced > this reversal of in and out ports ? I'm sure they are reversed. This patch was also tested by Jim Schutt. I didn't check which commit introduced this - why is it important?
> >> For optimal sl2vl added override of default ALL settting with port's >> sl2vl when operational VL was other than the default port. > > What is the motivation to override when the operational VLs is > different ? Why is that better than what is done currently ? The idea was to apply the default policy - set sl2vl modulo operational VL. When applying ALL settings using port 1 we still want to override this setting for ports with different operational VL. > > Is this really a policy issue ? > > IMO there are two separate issues in this patch and they should be in > separate patches (for better git bisection). Maybe, but I still think this patch fixes wrong setting for sl2vl using optimized and non optimized methods. I'm not sure this should be divided to 2 separate patches. > > Also, a couple of (possibly related) questions below. It seems that you are referring to patch v1 which was modified according to Jim's comments. Please check the v2 patch . Thanks, Eli > >> Signed-off-by: Eli Dorfman <[email protected]> >> --- >> opensm/opensm/osm_qos.c | 25 ++++++++++++++++++++----- >> 1 files changed, 20 insertions(+), 5 deletions(-) >> >> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c >> index a571370..de0ae23 100644 >> --- a/opensm/opensm/osm_qos.c >> +++ b/opensm/opensm/osm_qos.c >> @@ -182,7 +182,7 @@ static ib_api_status_t sl2vl_update_table(osm_sm_t * sm, >> osm_physp_t * p, >> tbl.raw_vl_by_sl[i] = (vl1 << 4) | vl2; >> } >> >> - if (!force_update && (p_tbl = osm_physp_get_slvl_tbl(p, in_port)) && >> + if (!force_update && in_port && (p_tbl = osm_physp_get_slvl_tbl(p, >> in_port)) && >> !memcmp(p_tbl, &tbl, sizeof(tbl))) >> return IB_SUCCESS; > > Why exclude port 0 here ? Is it related to the change noted below ? > >> @@ -209,6 +209,7 @@ static int qos_extports_setup(osm_sm_t * sm, osm_node_t >> *node, >> unsigned num_ports = osm_node_get_num_physp(node); >> int ret = 0; >> unsigned i, j; >> + uint8_t op_vl1; >> >> for (i = 1; i < num_ports; i++) { >> p = osm_node_get_physp_ptr(node, i); >> @@ -225,17 +226,31 @@ static int qos_extports_setup(osm_sm_t * sm, >> osm_node_t *node, >> 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); >> + op_vl1 = ib_port_info_get_op_vls(&p->port_info); >> force_update = p->need_update || sm->p_subn->need_update; >> - return sl2vl_update_table(sm, p, 1, 0x30000, force_update, >> - &qcfg->sl2vl); >> + if (sl2vl_update_table(sm, p, 0, 0x30000, force_update, > > Why is the third parameter (in_port) changed from 1 to 0 here ? Maybe > that's related to the change above for the skipping of port 0 in > sl2vl_update_table. > > -- Hal > >> + &qcfg->sl2vl)) >> + ret = -1; >> + /* overwrite default ALL configuration if port's >> + op_vl is different */ >> + for (i = 2; i < num_ports; i++) { >> + p = osm_node_get_physp_ptr(node, i); >> + if (ib_port_info_get_op_vls(&p->port_info) != op_vl1 >> && >> + sl2vl_update_table(sm, p, 0, 0x20000 | i, >> force_update, >> + &qcfg->sl2vl)) >> + ret = -1; >> + } >> + return ret; >> } >> >> - for (i = 0; i < num_ports; i++) { >> + /* non optimized sl2vl configuration */ >> + i = ib_switch_info_is_enhanced_port0(&node->sw->switch_info) ? 0 : 1; >> + for (; i < num_ports; i++) { >> p = osm_node_get_physp_ptr(node, i); >> force_update = p->need_update || sm->p_subn->need_update; >> j = ib_switch_info_is_enhanced_port0(&node->sw->switch_info) >> ? 0 : 1; >> for (; j < num_ports; j++) >> - if (sl2vl_update_table(sm, p, i, i << 8 | j, >> + if (sl2vl_update_table(sm, p, j, j << 8 | i, >> force_update, &qcfg->sl2vl)) >> ret = -1; >> } >> -- >> 1.5.5 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in >> the body of a message to [email protected] >> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
