[ofa-general] Re: [PATCH] opensm/osm_link_mgr.c initialize SMSL

[Line.Holen at Sun.COM]

On 03/ 6/09 11:56 AM, Sasha Khapyorsky wrote:
> Hi Line,
>
> On 10:27 Wed 04 Mar     , Line.Holen at Sun.COM wrote:
>   
>> The SMSL is currently not initialized. Whatever is written in PortInfo
>> at power up is kept.
>>
>> The lash routing algorithm makes active use of SL to avoid deadlock in
>> the fabric. The endnodes needs to be instructed to use the SL calculated
>> for the endnode-to-SMnode path for SA traffic. If not, the SA traffic
>> can cause deadlock.
>>
>> This change makes sure that the SMSL is properly initialized on all
>> ports. If flash routing is used, the SMSL is set up based on routing
>> calculations. For other routing algorithms the default SMSL is used.
>>
>> Signed-off-by: Line Holen <line.holen at sun.com>
>>     
>
> I cannot even try to apply the patch - all white spaces are mangled
> there. Please setup your mailer. (For tips you can see MUA sections of
> http://git.kernel.org/?p=git/git.git;a=blob_plain;f=Documentation/SubmittingPatches)
>   
OK, I'll look into this. BTW, is there any policy with respect to use of 
tab for indentations ?
>   
>> ---
>>
>> diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
>> index 37e3e1b..44765e3 100644
>> --- a/opensm/opensm/osm_link_mgr.c
>> +++ b/opensm/opensm/osm_link_mgr.c
>> @@ -2,6 +2,7 @@
>>  * Copyright (c) 2004-2008 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 Sun Microsystems, Inc. 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
>> @@ -51,6 +52,49 @@
>> #include <opensm/osm_switch.h>
>> #include <opensm/osm_helper.h>
>> #include <opensm/osm_msgdef.h>
>> +#include <opensm/osm_opensm.h>
>> +
>> +extern uint8_t osm_get_lash_sl(osm_opensm_t * p_osm,
>> +                   const osm_port_t * p_src_port,
>> +                   const osm_port_t * p_dst_port);
>>     
>
> This function is used in more than one places, so it can be useful to
> put its prototype in header file (osm_ucast_lash.h for example).
>   
OK.
>   
>> +
>> +/**********************************************************************
>> + **********************************************************************/
>> +static uint8_t
>> +__osm_link_mgr_get_smsl(IN osm_sm_t * sm,
>> +            IN osm_physp_t * const p_physp)
>>     
>
> '__osm_' prefix is not really needed for static functions.
>   
This prefix is used for all the other static functions in this file, but 
I can remove it
if you like.
>   
>> +{
>> +    osm_opensm_t     *p_osm = sm->p_subn->p_osm;
>> +    const osm_port_t *p_sm_port;
>> +    const osm_port_t *p_src_port;
>> +    ib_net16_t     slid;
>> +    ib_net16_t     smlid;
>> +    uint8_t         sl;
>> +
>> +    OSM_LOG_ENTER(sm->p_log);
>> +
>> +    if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH) {
>> +        /* Use default SL if lash routing is not used */
>> +        OSM_LOG_EXIT(sm->p_log);
>> +        return (OSM_DEFAULT_SL);
>> +    }
>> +
>> +    /* Find osm_port of the SM itself = dest_port */
>> +    smlid = sm->p_subn->sm_base_lid;
>> +    p_sm_port = cl_ptr_vector_get(&sm->p_subn->port_lid_tbl, 
>> cl_ntoh16(smlid));
>> +
>> +    /* Find osm_port of the source = p_physp */
>> +    slid = osm_physp_get_base_lid(p_physp);
>> +    p_src_port = cl_ptr_vector_get(&sm->p_subn->port_lid_tbl, 
>> cl_ntoh16(slid));
>> +
>> +    /* Call lash to find proper SL */
>> +    sl = osm_get_lash_sl(p_osm, p_src_port, p_sm_port);
>> +    OSM_LOG(sm->p_log, OSM_LOG_DEBUG, "SL for LID %u to SA is %d\n",
>> +        cl_ntoh16(slid), sl);
>>     
>
> Please think again do we really need such noisy debug message?
>   
It can be removed.
> The rest looks fine. Although it would be nice to not duplicate code for
> cases of base port 0 and other ports.
>
> Sasha
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>   

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20090306/06fdf7a5/attachment.html>