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

[Sasha Khapyorsky]

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)

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

> +
> +/**********************************************************************
> + **********************************************************************/
> +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.

> +{
> +    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?

The rest looks fine. Although it would be nice to not duplicate code for
cases of base port 0 and other ports.

Sasha