[ofa-general] Re: [PATCH] osm: QoS - adding new QoS fields to MultiPathRecord
Yevgeny Kliteynik
kliteyn at dev.mellanox.co.il
Tue Sep 4 12:54:05 PDT 2007
Sasha Khapyorsky wrote:
> On 18:17 Tue 04 Sep , Yevgeny Kliteynik wrote:
>> Hi Sasha,
>>
>> Adding QoS class and Service ID to MultiPathRecord
>>
>> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>
> Applied. Thanks.
>
> The nit is below.
>
>> ---
>> opensm/include/iba/ib_types.h | 181 +++++++++++++++++++++++++++++--
>> opensm/libvendor/osm_vendor_ibumad_sa.c | 3 +-
>> opensm/opensm/osm_helper.c | 13 ++-
>> 3 files changed, 180 insertions(+), 17 deletions(-)
>>
>> diff --git a/opensm/include/iba/ib_types.h b/opensm/include/iba/ib_types.h
>> index 0a096f9..13e2f38 100644
>> --- a/opensm/include/iba/ib_types.h
>> +++ b/opensm/include/iba/ib_types.h
>> @@ -1658,6 +1658,17 @@ static inline boolean_t OSM_API ib_class_is_rmpp(IN const uint8_t class_code)
>> */
>> #define IB_PATH_REC_SL_MASK 0x000F
>>
>> +/****d* IBA Base: Constants/IB_MULTIPATH_REC_SL_MASK
>> +* NAME
>> +* IB_MILTIPATH_REC_SL_MASK
>> +*
>> +* DESCRIPTION
>> +* Mask for the sl field for MultiPath record
>> +*
>> +* SOURCE
>> +*/
>> +#define IB_MULTIPATH_REC_SL_MASK 0x000F
>> +
>> /****d* IBA Base: Constants/IB_PATH_REC_QOS_CLASS_MASK
>> * NAME
>> * IB_PATH_REC_QOS_CLASS_MASK
>> @@ -1669,6 +1680,17 @@ static inline boolean_t OSM_API ib_class_is_rmpp(IN const uint8_t class_code)
>> */
>> #define IB_PATH_REC_QOS_CLASS_MASK 0xFFF0
>>
>> +/****d* IBA Base: Constants/IB_MULTIPATH_REC_QOS_CLASS_MASK
>> +* NAME
>> +* IB_MULTIPATH_REC_QOS_CLASS_MASK
>> +*
>> +* DESCRIPTION
>> +* Mask for the QoS class field for MultiPath record
>> +*
>> +* SOURCE
>> +*/
>> +#define IB_MULTIPATH_REC_QOS_CLASS_MASK 0xFFF0
>> +
>> /****d* IBA Base: Constants/IB_PATH_REC_SELECTOR_MASK
>> * NAME
>> * IB_PATH_REC_SELECTOR_MASK
>> @@ -2589,7 +2611,7 @@ typedef struct _ib_path_rec {
>> #define IB_MPR_COMPMASK_REVERSIBLE (CL_HTON64(((uint64_t)1)<<5))
>> #define IB_MPR_COMPMASK_NUMBPATH (CL_HTON64(((uint64_t)1)<<6))
>> #define IB_MPR_COMPMASK_PKEY (CL_HTON64(((uint64_t)1)<<7))
>> -#define IB_MPR_COMPMASK_RESV1 (CL_HTON64(((uint64_t)1)<<8))
>> +#define IB_MPR_COMPMASK_QOS_CLASS (CL_HTON64(((uint64_t)1)<<8))
>> #define IB_MPR_COMPMASK_SL (CL_HTON64(((uint64_t)1)<<9))
>> #define IB_MPR_COMPMASK_MTUSELEC (CL_HTON64(((uint64_t)1)<<10))
>> #define IB_MPR_COMPMASK_MTU (CL_HTON64(((uint64_t)1)<<11))
>> @@ -2597,12 +2619,12 @@ typedef struct _ib_path_rec {
>> #define IB_MPR_COMPMASK_RATE (CL_HTON64(((uint64_t)1)<<13))
>> #define IB_MPR_COMPMASK_PKTLIFETIMESELEC (CL_HTON64(((uint64_t)1)<<14))
>> #define IB_MPR_COMPMASK_PKTLIFETIME (CL_HTON64(((uint64_t)1)<<15))
>> -#define IB_MPR_COMPMASK_RESV2 (CL_HTON64(((uint64_t)1)<<16))
>> +#define IB_MPR_COMPMASK_SERVICEID_MSB (CL_HTON64(((uint64_t)1)<<16))
>> #define IB_MPR_COMPMASK_INDEPSELEC (CL_HTON64(((uint64_t)1)<<17))
>> #define IB_MPR_COMPMASK_RESV3 (CL_HTON64(((uint64_t)1)<<18))
>> #define IB_MPR_COMPMASK_SGIDCOUNT (CL_HTON64(((uint64_t)1)<<19))
>> #define IB_MPR_COMPMASK_DGIDCOUNT (CL_HTON64(((uint64_t)1)<<20))
>> -#define IB_MPR_COMPMASK_RESV4 (CL_HTON64(((uint64_t)1)<<21))
>> +#define IB_MPR_COMPMASK_SERVICEID_LSB (CL_HTON64(((uint64_t)1)<<21))
>>
>> /* SMInfo Record Component Masks */
>> #define IB_SMIR_COMPMASK_LID (CL_HTON64(((uint64_t)1)<<0))
>> @@ -5861,16 +5883,15 @@ typedef struct _ib_multipath_rec_t {
>> uint8_t tclass;
>> uint8_t num_path;
>> ib_net16_t pkey;
>> - uint8_t resv0;
>> - uint8_t sl;
>> + ib_net16_t qos_class_sl;
>> uint8_t mtu;
>> uint8_t rate;
>> uint8_t pkt_life;
>> - uint8_t resv1;
>> + uint8_t service_id_8msb;
>> uint8_t independence; /* formerly resv2 */
>> uint8_t sgid_count;
>> uint8_t dgid_count;
>> - uint8_t resv3[7];
>> + uint8_t service_id_56lsb[7];
>> ib_gid_t gids[IB_MULTIPATH_MAX_GIDS];
>> } PACK_SUFFIX ib_multipath_rec_t;
>> #include <complib/cl_packoff.h>
>> @@ -5890,8 +5911,8 @@ typedef struct _ib_multipath_rec_t {
>> * pkey
>> * Partition key (P_Key) to use on this path.
>> *
>> -* sl
>> -* Service level to use on this path.
>> +* qos_class_sl
>> +* QoS class and service level to use on this path.
>> *
>> * mtu
>> * MTU and MTU selector fields to use on this path
>> @@ -5901,6 +5922,12 @@ typedef struct _ib_multipath_rec_t {
>> * pkt_life
>> * Packet lifetime
>> *
>> +* service_id_8msb
>> +* 8 most significant bits of Service ID
>> +*
>> +* service_id_56lsb
>> +* 56 least significant bits of Service ID
>> +*
>> * preference
>> * Indicates the relative merit of this path versus other path
>> * records returned from the SA. Lower numbers are better.
>> @@ -5937,6 +5964,41 @@ ib_multipath_rec_num_path(IN const ib_multipath_rec_t * const p_rec)
>> * ib_multipath_rec_t
>> *********/
>>
>> +/****f* IBA Base: Types/ib_multipath_rec_set_sl
>> +* NAME
>> +* ib_multipath_rec_set_sl
>> +*
>> +* DESCRIPTION
>> +* Set path service level.
>> +*
>> +* SYNOPSIS
>> +*/
>> +static inline void OSM_API
>> +ib_multipath_rec_set_sl(
>> + IN ib_multipath_rec_t* const p_rec,
>> + IN const uint8_t sl )
>> +{
>> + p_rec->qos_class_sl =
>> + (p_rec->qos_class_sl & CL_HTON16(IB_MULTIPATH_REC_QOS_CLASS_MASK)) |
>> + cl_hton16(sl & IB_MULTIPATH_REC_SL_MASK);
>> +}
>> +/*
>> +* PARAMETERS
>> +* p_rec
>> +* [in] Pointer to the MultiPath record object.
>> +*
>> +* sl
>> +* [in] Service level to set.
>> +*
>> +* RETURN VALUES
>> +* None
>> +*
>> +* NOTES
>> +*
>> +* SEE ALSO
>> +* ib_multipath_rec_t
>> +*********/
>> +
>> /****f* IBA Base: Types/ib_multipath_rec_sl
>> * NAME
>> * ib_multipath_rec_sl
>> @@ -5949,7 +6011,7 @@ ib_multipath_rec_num_path(IN const ib_multipath_rec_t * const p_rec)
>> static inline uint8_t OSM_API
>> ib_multipath_rec_sl(IN const ib_multipath_rec_t * const p_rec)
>> {
>> - return ((uint8_t) ((cl_ntoh16(p_rec->sl)) & 0xF));
>> + return ((uint8_t) ((cl_ntoh16(p_rec->qos_class_sl)) & IB_MULTIPATH_REC_SL_MASK));
>> }
>>
>> /*
>> @@ -5966,6 +6028,70 @@ ib_multipath_rec_sl(IN const ib_multipath_rec_t * const p_rec)
>> * ib_multipath_rec_t
>> *********/
>>
>> +/****f* IBA Base: Types/ib_multipath_rec_set_qos_class
>> +* NAME
>> +* ib_multipath_rec_set_qos_class
>> +*
>> +* DESCRIPTION
>> +* Set path QoS class.
>> +*
>> +* SYNOPSIS
>> +*/
>> +static inline void OSM_API
>> +ib_multipath_rec_set_qos_class(
>> + IN ib_multipath_rec_t* const p_rec,
>> + IN const uint16_t qos_class )
>> +{
>> + p_rec->qos_class_sl =
>> + (p_rec->qos_class_sl & CL_HTON16(IB_MULTIPATH_REC_SL_MASK)) |
>> + cl_hton16(qos_class << 4);
>> +}
>> +/*
>> +* PARAMETERS
>> +* p_rec
>> +* [in] Pointer to the MultiPath record object.
>> +*
>> +* qos_class
>> +* [in] QoS class to set.
>> +*
>> +* RETURN VALUES
>> +* None
>> +*
>> +* NOTES
>> +*
>> +* SEE ALSO
>> +* ib_multipath_rec_t
>> +*********/
>> +
>> +/****f* IBA Base: Types/ib_multipath_rec_qos_class
>> +* NAME
>> +* ib_multipath_rec_qos_class
>> +*
>> +* DESCRIPTION
>> +* Get QoS class.
>> +*
>> +* SYNOPSIS
>> +*/
>> +static inline uint16_t OSM_API
>> +ib_multipath_rec_qos_class(
>> + IN const ib_multipath_rec_t* const p_rec )
>> +{
>> + return (cl_ntoh16( p_rec->qos_class_sl ) >> 4);
>> +}
>> +/*
>> +* PARAMETERS
>> +* p_rec
>> +* [in] Pointer to the MultiPath record object.
>> +*
>> +* RETURN VALUES
>> +* QoS class of the MultiPath record.
>> +*
>> +* NOTES
>> +*
>> +* SEE ALSO
>> +* ib_multipath_rec_t
>> +*********/
>> +
>> /****f* IBA Base: Types/ib_multipath_rec_mtu
>> * NAME
>> * ib_multipath_rec_mtu
>> @@ -6164,6 +6290,41 @@ ib_multipath_rec_pkt_life_sel(IN const ib_multipath_rec_t * const p_rec)
>> * ib_multipath_rec_t
>> *********/
>>
>> +/****f* IBA Base: Types/ib_multipath_rec_service_id
>> +* NAME
>> +* ib_multipath_rec_service_id
>> +*
>> +* DESCRIPTION
>> +* Get multipath service id.
>> +*
>> +* SYNOPSIS
>> +*/
>> +static inline uint64_t OSM_API
>> +ib_multipath_rec_service_id(IN const ib_multipath_rec_t * const p_rec)
>> +{
>> + union {
>> + ib_net64_t sid;
>> + uint8_t sid_arr[8];
>> + } sid_union;
>> + sid_union.sid_arr[0] = p_rec->service_id_8msb;
>> + memcpy(&sid_union.sid_arr[1], p_rec->service_id_56lsb, 7);
>> + return sid_union.sid;
>> +}
>> +
>> +/*
>> +* PARAMETERS
>> +* p_rec
>> +* [in] Pointer to the multipath record object.
>> +*
>> +* RETURN VALUES
>> +* Service ID
>> +*
>> +* NOTES
>> +*
>> +* SEE ALSO
>> +* ib_multipath_rec_t
>> +*********/
>> +
>> #define IB_NUM_PKEY_ELEMENTS_IN_BLOCK 32
>> /****s* IBA Base: Types/ib_pkey_table_t
>> * NAME
>> diff --git a/opensm/libvendor/osm_vendor_ibumad_sa.c b/opensm/libvendor/osm_vendor_ibumad_sa.c
>> index 42a6d3a..a878c71 100644
>> --- a/opensm/libvendor/osm_vendor_ibumad_sa.c
>> +++ b/opensm/libvendor/osm_vendor_ibumad_sa.c
>> @@ -840,7 +840,8 @@ osmv_query_sa(IN osm_bind_handle_t h_bind,
>> else
>> multipath_rec.num_path &= ~0x80;
>> multipath_rec.pkey = p_mpr_req->pkey;
>> - multipath_rec.sl = p_mpr_req->sl;
>> + ib_multipath_rec_set_sl(&multipath_rec, p_mpr_req->sl);
>> + ib_multipath_rec_set_qos_class(&multipath_rec, 0);
>> multipath_rec.independence = p_mpr_req->independence;
>> multipath_rec.sgid_count = p_mpr_req->sgid_count;
>> multipath_rec.dgid_count = p_mpr_req->dgid_count;
>> diff --git a/opensm/opensm/osm_helper.c b/opensm/opensm/osm_helper.c
>> index 5dd3955..cf8cfab 100644
>> --- a/opensm/opensm/osm_helper.c
>> +++ b/opensm/opensm/osm_helper.c
>> @@ -1131,29 +1131,30 @@ osm_dump_multipath_record(IN osm_log_t * const p_log,
>> "\t\t\t\ttclass..................0x%X\n"
>> "\t\t\t\tnum_path_revers.........0x%X\n"
>> "\t\t\t\tpkey....................0x%X\n"
>> - "\t\t\t\tresv0...................0x%X\n"
>> + "\t\t\t\tqos_class...............0x%X\n"
>> "\t\t\t\tsl......................0x%X\n"
>> "\t\t\t\tmtu.....................0x%X\n"
>> "\t\t\t\trate....................0x%X\n"
>> "\t\t\t\tpkt_life................0x%X\n"
>> - "\t\t\t\tresv1...................0x%X\n"
>> "\t\t\t\tindependence............0x%X\n"
>> "\t\t\t\tsgid_count..............0x%X\n"
>> "\t\t\t\tdgid_count..............0x%X\n"
>> + "\t\t\t\tservice_id..............0x%016" PRIx64 "\n"
>> "%s\n"
>> "",
>> cl_ntoh32(p_mpr->hop_flow_raw),
>> p_mpr->tclass,
>> p_mpr->num_path,
>> cl_ntoh16(p_mpr->pkey),
>> - p_mpr->resv0,
>> - cl_ntoh16(p_mpr->sl),
>> + ib_multipath_rec_qos_class(p_mpr),
>> + ib_multipath_rec_sl(p_mpr),
>> p_mpr->mtu,
>> p_mpr->rate,
>> p_mpr->pkt_life,
>> - p_mpr->resv1,
>> p_mpr->independence,
>> - p_mpr->sgid_count, p_mpr->dgid_count, buf_line);
>> + p_mpr->sgid_count, p_mpr->dgid_count,
>> + ib_multipath_rec_service_id(p_mpr),
>
> It returns serveice_id in network byte order. Should cl_ntoh64() be
> here?
Right.
Actually, this error is here because ib_multipath_rec_service_id() was
originally returning Service ID in host order, and then I changed it to
network order. No particular reason - couldn't decide which one to choose.
What do you think?
-- Yevgeny
> Sasha
>
>> + buf_line);
>> }
>> }
>>
>> --
>> 1.5.1.4
>>
>>
>
More information about the general
mailing list