diff mbox

opensm: update internal PortInfo state for any ports

Message ID 20130707205219.GA6084@gmail.com (mailing list archive)
State Superseded
Delegated to: Hal Rosenstock
Headers show

Commit Message

Sasha Khapyorsky July 7, 2013, 8:56 p.m. UTC
Should not be matter to keep internal SM's PortInfo data for ports
in any states.

Signed-off-by: Sasha Khapyorsky <sashakh@gmail.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

Hal Rosenstock July 12, 2013, 1:37 p.m. UTC | #1
On 7/7/2013 4:56 PM, Sasha Khapyorsky wrote:
> Should not be matter to keep internal SM's PortInfo data for ports
> in any states.

What difference does it make ? Does this patch fix some operational
issue ? Is there any harm in not keeping the complete PortInfo when port
is DOWN ?

At worst, the current implementation is a minor efficiency improvement:
to only copy the 2 required to be valid fields rather than all 64 bytes
in PortInfo. Also, it seems safer to only save the guaranteed valid fields.

-- Hal

> Signed-off-by: Sasha Khapyorsky <sashakh@gmail.com>
> 
> 
> diff --git a/opensm/osm_port.c b/opensm/osm_port.c
> index 6e73e66..d59d404 100644
> --- a/opensm/osm_port.c
> +++ b/opensm/osm_port.c
> @@ -669,25 +669,16 @@ void osm_physp_set_port_info(IN osm_physp_t * p_physp,
>  	CL_ASSERT(p_pi);
>  	CL_ASSERT(osm_physp_is_valid(p_physp));
>  
> -	if (ib_port_info_get_port_state(p_pi) == IB_LINK_DOWN) {
> -		/* If PortState is down, only copy PortState */
> -		/* and PortPhysicalState per C14-24-2.1 */
> -		ib_port_info_set_port_state(&p_physp->port_info, IB_LINK_DOWN);
> -		ib_port_info_set_port_phys_state
> -		    (ib_port_info_get_port_phys_state(p_pi),
> -		     &p_physp->port_info);
> -	} else {
> -		p_physp->port_info = *p_pi;
> -
> -		/* The MKey in p_pi can only be considered valid if it's
> -		 * for a HCA/router or switch port 0, and it's either
> -		 * non-zero or the MKeyProtect bits are also zero.
> -		 */
> -		if ((osm_node_get_type(p_physp->p_node) !=
> -		     IB_NODE_TYPE_SWITCH || p_physp->port_num == 0) &&
> -		    (p_pi->m_key != 0 || ib_port_info_get_mpb(p_pi) == 0))
> -			osm_db_guid2mkey_set(p_sm->p_subn->p_g2m,
> -					     cl_ntoh64(p_physp->port_guid),
> -					     cl_ntoh64(p_pi->m_key));
> -	}
> +	p_physp->port_info = *p_pi;
> +
> +	/* The MKey in p_pi can only be considered valid if it's
> +	 * for a HCA/router or switch port 0, and it's either
> +	 * non-zero or the MKeyProtect bits are also zero.
> +	 */
> +	if ((osm_node_get_type(p_physp->p_node) != IB_NODE_TYPE_SWITCH ||
> +	     p_physp->port_num == 0) &&
> +	    (p_pi->m_key != 0 || ib_port_info_get_mpb(p_pi) == 0))
> +		osm_db_guid2mkey_set(p_sm->p_subn->p_g2m,
> +				     cl_ntoh64(p_physp->port_guid),
> +				     cl_ntoh64(p_pi->m_key));
>  }
> 

--
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
Sasha Khapyorsky July 22, 2013, 3:40 p.m. UTC | #2
Hi Hal,

On 09:37 Fri 12 Jul     , Hal Rosenstock wrote:
> 
> What difference does it make ? Does this patch fix some operational
> issue ? Is there any harm in not keeping the complete PortInfo when port
> is DOWN ?

AFAIK it doesn't harm in the current state. However, I'm prepering the
patches, which will let to OpenSM work on stand alone port. For such
mode this fix is critical.

Will recent with other patches shortly.

Sasha
--
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 Aug. 8, 2013, 3:03 p.m. UTC | #3
Hi Sasha,

On 7/22/2013 11:40 AM, Sasha Khapyorsky wrote:
> Hi Hal,
> 
> On 09:37 Fri 12 Jul     , Hal Rosenstock wrote:
>>
>> What difference does it make ? Does this patch fix some operational
>> issue ? Is there any harm in not keeping the complete PortInfo when port
>> is DOWN ?
> 
> AFAIK it doesn't harm in the current state. However, I'm prepering the
> patches, which will let to OpenSM work on stand alone port. For such
> mode this fix is critical.

Why is it critical for this set of patches ? I'm still missing why
that's required for supporting single port sweep (so loopback
connections still work). What PortInfo fields are really needed ?

-- Hal

> Will recent with other patches shortly.
> 
> Sasha
> .
> 

--
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_port.c b/opensm/osm_port.c
index 6e73e66..d59d404 100644
--- a/opensm/osm_port.c
+++ b/opensm/osm_port.c
@@ -669,25 +669,16 @@  void osm_physp_set_port_info(IN osm_physp_t * p_physp,
 	CL_ASSERT(p_pi);
 	CL_ASSERT(osm_physp_is_valid(p_physp));
 
-	if (ib_port_info_get_port_state(p_pi) == IB_LINK_DOWN) {
-		/* If PortState is down, only copy PortState */
-		/* and PortPhysicalState per C14-24-2.1 */
-		ib_port_info_set_port_state(&p_physp->port_info, IB_LINK_DOWN);
-		ib_port_info_set_port_phys_state
-		    (ib_port_info_get_port_phys_state(p_pi),
-		     &p_physp->port_info);
-	} else {
-		p_physp->port_info = *p_pi;
-
-		/* The MKey in p_pi can only be considered valid if it's
-		 * for a HCA/router or switch port 0, and it's either
-		 * non-zero or the MKeyProtect bits are also zero.
-		 */
-		if ((osm_node_get_type(p_physp->p_node) !=
-		     IB_NODE_TYPE_SWITCH || p_physp->port_num == 0) &&
-		    (p_pi->m_key != 0 || ib_port_info_get_mpb(p_pi) == 0))
-			osm_db_guid2mkey_set(p_sm->p_subn->p_g2m,
-					     cl_ntoh64(p_physp->port_guid),
-					     cl_ntoh64(p_pi->m_key));
-	}
+	p_physp->port_info = *p_pi;
+
+	/* The MKey in p_pi can only be considered valid if it's
+	 * for a HCA/router or switch port 0, and it's either
+	 * non-zero or the MKeyProtect bits are also zero.
+	 */
+	if ((osm_node_get_type(p_physp->p_node) != IB_NODE_TYPE_SWITCH ||
+	     p_physp->port_num == 0) &&
+	    (p_pi->m_key != 0 || ib_port_info_get_mpb(p_pi) == 0))
+		osm_db_guid2mkey_set(p_sm->p_subn->p_g2m,
+				     cl_ntoh64(p_physp->port_guid),
+				     cl_ntoh64(p_pi->m_key));
 }