Message ID | 20130707205219.GA6084@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Hal Rosenstock |
Headers | show |
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
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
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 --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)); }
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