diff mbox

[1/2] opensm/osm_state_mgr.c: Don't rely on PortInfo:PortState for base SP0

Message ID 4DAC3CCC.4090101@dev.mellanox.co.il (mailing list archive)
State Accepted
Delegated to: Alex Netes
Headers show

Commit Message

Hal Rosenstock April 18, 2011, 1:29 p.m. UTC
For base SP0, PortState in SM PortInfo attribute is not used and base SP0
is always "active". 

Signed-off-by: Hal Rosenstock <hal@mellanox.com>
---
--
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

Comments

Alex Netes April 20, 2011, 2:17 p.m. UTC | #1
Hi Hal,

On 09:29 Mon 18 Apr     , Hal Rosenstock wrote:
> 
> For base SP0, PortState in SM PortInfo attribute is not used and base SP0
> is always "active". 

How SM can be attached to base SP0?
Moreover, if during discovery we found that SM is attached to base SP0, don't
we want to act like the port is down?

> 
> Signed-off-by: Hal Rosenstock <hal@mellanox.com>
> ---
> diff --git a/opensm/osm_state_mgr.c b/opensm/osm_state_mgr.c
> index 2e1ef94..dd308f2 100644
> --- a/opensm/osm_state_mgr.c
> +++ b/opensm/osm_state_mgr.c
> @@ -352,7 +352,11 @@ static boolean_t state_mgr_is_sm_port_down(IN osm_sm_t * sm)
>  
>  	CL_ASSERT(p_physp);
>  
> -	state = osm_physp_get_port_state(p_physp);
> +	if (p_port->p_node->sw &&
> +	    !ib_switch_info_is_enhanced_port0(&p_port->p_node->sw->switch_info))
> +		state = IB_LINK_ACTIVE;	/* base SP0 */
> +	else
> +		state = osm_physp_get_port_state(p_physp);
>  	CL_PLOCK_RELEASE(sm->p_lock);
>  
>  Exit:
> --
> 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

-- Alex
--
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
Hal Rosenstock April 20, 2011, 2:43 p.m. UTC | #2
Hi Alex,

On 4/20/2011 10:17 AM, Alex Netes wrote:
> Hi Hal,
> 
> On 09:29 Mon 18 Apr     , Hal Rosenstock wrote:
>>
>> For base SP0, PortState in SM PortInfo attribute is not used and base SP0
>> is always "active". 
> 
> How SM can be attached to base SP0?

What precludes this ? AFAIK SM can run on any switch port 0 whether
enhanced or base.

> Moreover, if during discovery we found that SM is attached to base SP0, don't
> we want to act like the port is down?

For base SP0, PortState and PortPhysicalState are "not used" in the SM
PortInfo attribute. That means they're "vendor" specific and they should
not be relied upon to be valid.

The point of this patch is that base SP0 is always up regardless of what
PortInfo for that port says.

If SM wasn't allowed to run on base SP0, it would be a different patch:
to prohibit such a port from being used at all as the SM port so the
failure would be earlier in the code flow when the SM tries to bind to
such a port.

-- Hal

>>
>> Signed-off-by: Hal Rosenstock <hal@mellanox.com>
>> ---
>> diff --git a/opensm/osm_state_mgr.c b/opensm/osm_state_mgr.c
>> index 2e1ef94..dd308f2 100644
>> --- a/opensm/osm_state_mgr.c
>> +++ b/opensm/osm_state_mgr.c
>> @@ -352,7 +352,11 @@ static boolean_t state_mgr_is_sm_port_down(IN osm_sm_t * sm)
>>  
>>  	CL_ASSERT(p_physp);
>>  
>> -	state = osm_physp_get_port_state(p_physp);
>> +	if (p_port->p_node->sw &&
>> +	    !ib_switch_info_is_enhanced_port0(&p_port->p_node->sw->switch_info))
>> +		state = IB_LINK_ACTIVE;	/* base SP0 */
>> +	else
>> +		state = osm_physp_get_port_state(p_physp);
>>  	CL_PLOCK_RELEASE(sm->p_lock);
>>  
>>  Exit:
>> --

> 
> -- Alex


--
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
Alex Netes April 21, 2011, 12:22 p.m. UTC | #3
On 10:43 Wed 20 Apr     , Hal Rosenstock wrote:
> Hi Alex,
> 
> On 4/20/2011 10:17 AM, Alex Netes wrote:
> > Hi Hal,
> > 
> > On 09:29 Mon 18 Apr     , Hal Rosenstock wrote:
> >>
> >> For base SP0, PortState in SM PortInfo attribute is not used and base SP0
> >> is always "active". 
> > 
> > How SM can be attached to base SP0?
> 
> What precludes this ? AFAIK SM can run on any switch port 0 whether
> enhanced or base.
> 

I'm probably missing something. Base Switch Port 0 - A Switch Port 0 which is
not an Endport (IBTA p71). 
If you look further, Endport defined as a Port which can be a destination of 
LID-routed communication within the same Subnet as the sender.

How can SM be attached to a port which isn't an Endport?

-- Alex
--
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
Hal Rosenstock April 21, 2011, 12:48 p.m. UTC | #4
On 4/21/2011 8:22 AM, Alex Netes wrote:
> On 10:43 Wed 20 Apr     , Hal Rosenstock wrote:
>> Hi Alex,
>>
>> On 4/20/2011 10:17 AM, Alex Netes wrote:
>>> Hi Hal,
>>>
>>> On 09:29 Mon 18 Apr     , Hal Rosenstock wrote:
>>>>
>>>> For base SP0, PortState in SM PortInfo attribute is not used and base SP0
>>>> is always "active". 
>>>
>>> How SM can be attached to base SP0?
>>
>> What precludes this ? AFAIK SM can run on any switch port 0 whether
>> enhanced or base.
>>
> 
> I'm probably missing something. Base Switch Port 0 - A Switch Port 0 which is
> not an Endport (IBTA p71). 

The glossary is only informative (and not compliance) and has a number
of such issues.

You can see that the Endport definition on p.73 contradicts this:
"All Channel Adapter ports on the subnet are
endports of that subnet, as is Port 0 of each Switch in the subnet.
Switch ports other than Port 0 may not be endports."

A spec comment (RefID 4352) was entered quite some time ago on this to
change this to something like:

A <Switch Port 0> with a set of basic capabilities (defined in this
specification). That set is less than the capabilities of an <Enhanced
Switch Port 0>, and also less than the minimum set of capabilities
required of a <Target Channel Adapter>.

> If you look further, Endport defined as a Port which can be a destination of 
> LID-routed communication within the same Subnet as the sender.

Actually this is imprecise as well as the phrase "within the same Subnet
as the sender" should be eliminated.

> How can SM be attached to a port which isn't an Endport?

Base SP0 does have a LID (look at PortInfo:LID Used By column is X'd).

-- Hal

> -- Alex
> --
> 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
Alex Netes April 27, 2011, 4:09 p.m. UTC | #5
Hi Hal,

On 09:29 Mon 18 Apr     , Hal Rosenstock wrote:
> 
> For base SP0, PortState in SM PortInfo attribute is not used and base SP0
> is always "active". 
> 
> Signed-off-by: Hal Rosenstock <hal@mellanox.com>
> ---

Applied. Thanks.
--
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
diff mbox

Patch

diff --git a/opensm/osm_state_mgr.c b/opensm/osm_state_mgr.c
index 2e1ef94..dd308f2 100644
--- a/opensm/osm_state_mgr.c
+++ b/opensm/osm_state_mgr.c
@@ -352,7 +352,11 @@  static boolean_t state_mgr_is_sm_port_down(IN osm_sm_t * sm)
 
 	CL_ASSERT(p_physp);
 
-	state = osm_physp_get_port_state(p_physp);
+	if (p_port->p_node->sw &&
+	    !ib_switch_info_is_enhanced_port0(&p_port->p_node->sw->switch_info))
+		state = IB_LINK_ACTIVE;	/* base SP0 */
+	else
+		state = osm_physp_get_port_state(p_physp);
 	CL_PLOCK_RELEASE(sm->p_lock);
 
 Exit: