public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] opensm/osm_link_mgr.c: In link_mgr_set_physp_pi, only call link_mgr_get_smsl when LID valid
       [not found]   ` <1830724148.4221541256937456735.JavaMail.root-m8vEhog2yEGs1RcX6exeh1Wh7bxQi8rYbcbDuTDi3LmGxX56iWlbXw@public.gmane.org>
@ 2009-10-30 23:31     ` Hal Rosenstock
       [not found]       ` <f0e08f230910301631x639a6670x8fbe184580964f9a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Hal Rosenstock @ 2009-10-30 23:31 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

Hi Sasha,

On Fri, Oct 30, 2009 at 5:17 PM, Sasha Khapyorsky <sashak@voltaire.com> wrote:
>
> Hi Hal,
>
> On 09:27 Wed 23 Sep     , Hal Rosenstock wrote:
>>
>> Fix seg fault which occurs when get_osm_switch_from_port is
>> called with NULL port (which in this case was caused by calling
>> cl_ptr_vector_get on port LID table with LID 0)
>
> Would be really useful to describe when and why this (LID = 0) can
> happen.

Thought we resolved this in "osm_link_mgr.c:link_mgr_get_smsl question" thread.

>>
>> Signed-off-by: Hal Rosenstock <hal.rosenstock@gmail.com>
>
> What about simpler change:
>
>
> diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
> index 4d4be56..217f51e 100644
> --- a/opensm/opensm/osm_link_mgr.c
> +++ b/opensm/opensm/osm_link_mgr.c
> @@ -68,7 +68,8 @@ static uint8_t link_mgr_get_smsl(IN osm_sm_t * sm, IN
> osm_physp_t * p_physp)
>
>          OSM_LOG_ENTER(sm->p_log);
>
> -        if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH) {
> +        if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH
> +            || !(slid = osm_physp_get_base_lid(p_physp))) {
>                  /* Use default SL if lash routing is not used */
>                  OSM_LOG_EXIT(sm->p_log);
>                  return (sm->p_subn->opt.sm_sl);
> @@ -80,7 +81,6 @@ static uint8_t link_mgr_get_smsl(IN osm_sm_t * sm, IN
> osm_physp_t * p_physp)
>              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));
>
>
> Wouldn't it be the same functionally?

Yes,

Do you want an updated patch ?

-- Hal

> Sasha
>
>> ---
>> diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
>> index c9bdfee..35f83e2 100644
>> --- a/opensm/opensm/osm_link_mgr.c
>> +++ b/opensm/opensm/osm_link_mgr.c
>> @@ -131,27 +131,32 @@ static int link_mgr_set_physp_pi(osm_sm_t * sm, IN
>> osm_physp_t * p_physp,
>>                  if
>> (ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info)
>>                      == FALSE) {
>>
>> -                        /* Even for base port 0 we might have to set smsl
>> -                           (if we are using lash routing) */
>> -                        smsl = link_mgr_get_smsl(sm, p_physp);
>> -                        if (smsl !=
>> ib_port_info_get_master_smsl(p_old_pi)) {
>> -                                send_set = TRUE;
>> -                                OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> -                                        "Setting SMSL to %d on port 0
>> GUID 0x%016"
>> -                                        PRIx64 "\n", smsl,
>> -                                        cl_ntoh64(osm_physp_get_port_guid
>> -                                                  (p_physp)));
>> -                        } else {
>> -                                /* This means the switch doesn't support
>> -                                   enhanced port 0 and we don't need to
>> -                                   change SMSL. Can skip it. */
>> -                                OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> -                                        "Skipping port 0, GUID 0x%016"
>> PRIx64
>> -                                        "\n",
>> -                                        cl_ntoh64(osm_physp_get_port_guid
>> -                                                  (p_physp)));
>> -                                goto Exit;
>> +                        /* Make sure LID is valid prior to calling
>> link_mgr_get_smsl */
>> +                        if (osm_physp_get_base_lid(p_physp)) {
>> +
>> +                                /* Even for base port 0 we might have to
>> set
>> +                                   smsl (if we are using lash routing) */
>> +                                smsl = link_mgr_get_smsl(sm, p_physp);
>> +                                if (smsl !=
>> ib_port_info_get_master_smsl(p_old_pi)) {
>> +                                        send_set = TRUE;
>> +                                        OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> +                                                "Setting SMSL to %d on
>> port 0 "
>> +                                                "GUID 0x%016" PRIx64
>> "\n", smsl,
>>
>> +                                                cl_ntoh64(osm_physp_get_port_guid
>> +                                                          (p_physp)));
>> +                                } else {
>> +                                        /* This means the switch doesn't
>> support
>> +                                           enhanced port 0 and we don't
>> need to
>> +                                           change SMSL. Can skip it. */
>> +                                        OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> +                                                "Skipping port 0, GUID
>> 0x%016"
>> +                                                PRIx64 "\n",
>>
>> +                                                cl_ntoh64(osm_physp_get_port_guid
>> +                                                          (p_physp)));
>> +                                        goto Exit;
>> +                                }
>>                          }
>> +
>>                  } else
>>                          esp0 = TRUE;
>>          }
>> @@ -217,18 +222,22 @@ static int link_mgr_set_physp_pi(osm_sm_t * sm, IN
>> osm_physp_t * p_physp,
>>                                     sizeof(p_pi->master_sm_base_lid)))
>>                                  send_set = TRUE;
>>
>> -                        smsl = link_mgr_get_smsl(sm, p_physp);
>> -                        if (smsl !=
>> ib_port_info_get_master_smsl(p_old_pi)) {
>> +                        /* Make sure LID is valid prior to calling
>> link_mgr_get_smsl */
>> +                        if (osm_physp_get_base_lid(p_physp)) {
>> +                                smsl = link_mgr_get_smsl(sm, p_physp);
>> +                                if (smsl !=
>> ib_port_info_get_master_smsl(p_old_pi)) {
>>
>> -                                ib_port_info_set_master_smsl(p_pi, smsl);
>>
>> +                                        ib_port_info_set_master_smsl(p_pi,
>> smsl);
>>
>> -                                OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> -                                        "Setting SMSL to %d on GUID
>> 0x%016"
>> -                                        PRIx64 ", port %d\n", smsl,
>> -                                        cl_ntoh64(osm_physp_get_port_guid
>> -                                                  (p_physp)), port_num);
>> +                                        OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> +                                                "Setting SMSL to %d on
>> GUID "
>> +                                                "0x%016" PRIx64 ", port
>> %d\n",
>> +                                                smsl,
>>
>> +                                                cl_ntoh64(osm_physp_get_port_guid
>> +                                                          (p_physp)),
>> port_num);
>>
>> -                                send_set = TRUE;
>> +                                        send_set = TRUE;
>> +                                }
>>                          }
>>
>>                          p_pi->m_key_lease_period =
>>
>

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

* Re: [PATCH] opensm/osm_link_mgr.c: In link_mgr_set_physp_pi, only call link_mgr_get_smsl when LID valid
       [not found]       ` <f0e08f230910301631x639a6670x8fbe184580964f9a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2009-10-31 12:12         ` Sasha Khapyorsky
  2009-10-31 12:13           ` [PATCH] opensm/link_mgr: verify port's lid Sasha Khapyorsky
  0 siblings, 1 reply; 3+ messages in thread
From: Sasha Khapyorsky @ 2009-10-31 12:12 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 19:31 Fri 30 Oct     , Hal Rosenstock wrote:
> Hi Sasha,
> 
> On Fri, Oct 30, 2009 at 5:17 PM, Sasha Khapyorsky <sashak-smomgflXvObQFizaE/u3fw@public.gmane.orgm> wrote:
> >
> > Hi Hal,
> >
> > On 09:27 Wed 23 Sep     , Hal Rosenstock wrote:
> >>
> >> Fix seg fault which occurs when get_osm_switch_from_port is
> >> called with NULL port (which in this case was caused by calling
> >> cl_ptr_vector_get on port LID table with LID 0)
> >
> > Would be really useful to describe when and why this (LID = 0) can
> > happen.
> 
> Thought we resolved this in "osm_link_mgr.c:link_mgr_get_smsl question" thread.

It is not for me. It is a change log and people which can read this
over years may not follow any mailing thread on the list.

> > diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
> > index 4d4be56..217f51e 100644
> > --- a/opensm/opensm/osm_link_mgr.c
> > +++ b/opensm/opensm/osm_link_mgr.c
> > @@ -68,7 +68,8 @@ static uint8_t link_mgr_get_smsl(IN osm_sm_t * sm, IN
> > osm_physp_t * p_physp)
> >
> >          OSM_LOG_ENTER(sm->p_log);
> >
> > -        if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH) {
> > +        if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH
> > +            || !(slid = osm_physp_get_base_lid(p_physp))) {
> >                  /* Use default SL if lash routing is not used */
> >                  OSM_LOG_EXIT(sm->p_log);
> >                  return (sm->p_subn->opt.sm_sl);
> > @@ -80,7 +81,6 @@ static uint8_t link_mgr_get_smsl(IN osm_sm_t * sm, IN
> > osm_physp_t * p_physp)
> >              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));
> >
> >
> > Wouldn't it be the same functionally?
> 
> Yes,
> 
> Do you want an updated patch ?

No need, I have it already.

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] 3+ messages in thread

* [PATCH] opensm/link_mgr: verify port's lid
  2009-10-31 12:12         ` Sasha Khapyorsky
@ 2009-10-31 12:13           ` Sasha Khapyorsky
  0 siblings, 0 replies; 3+ messages in thread
From: Sasha Khapyorsky @ 2009-10-31 12:13 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA


When resolving port by lid (in link_mgr_get_smsl()) be sure that lid is
valid. This can happen that during discovery PortInfo set fails after
lid manager run and port's local PortInfo still be zeroed.

The issue was pointed out and investigated by Hal Rosenstock.

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

diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
index c9bdfee..76325ea 100644
--- a/opensm/opensm/osm_link_mgr.c
+++ b/opensm/opensm/osm_link_mgr.c
@@ -68,7 +68,8 @@ static uint8_t link_mgr_get_smsl(IN osm_sm_t * sm, IN osm_physp_t * p_physp)
 
 	OSM_LOG_ENTER(sm->p_log);
 
-	if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH) {
+	if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH
+	    || !(slid = osm_physp_get_base_lid(p_physp))) {
 		/* Use default SL if lash routing is not used */
 		OSM_LOG_EXIT(sm->p_log);
 		return (sm->p_subn->opt.sm_sl);
@@ -80,7 +81,6 @@ static uint8_t link_mgr_get_smsl(IN osm_sm_t * sm, IN osm_physp_t * p_physp)
 	    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));
 
-- 
1.6.5.1

--
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] 3+ messages in thread

end of thread, other threads:[~2009-10-31 12:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20091030004732.GP20136@me>
     [not found] ` <1830724148.4221541256937456735.JavaMail.root@sz0074a.westchester.pa.mail.comcast.net>
     [not found]   ` <1830724148.4221541256937456735.JavaMail.root-m8vEhog2yEGs1RcX6exeh1Wh7bxQi8rYbcbDuTDi3LmGxX56iWlbXw@public.gmane.org>
2009-10-30 23:31     ` [PATCH] opensm/osm_link_mgr.c: In link_mgr_set_physp_pi, only call link_mgr_get_smsl when LID valid Hal Rosenstock
     [not found]       ` <f0e08f230910301631x639a6670x8fbe184580964f9a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-10-31 12:12         ` Sasha Khapyorsky
2009-10-31 12:13           ` [PATCH] opensm/link_mgr: verify port's lid Sasha Khapyorsky

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