[openib-general] [PATCH] osm: QoS: added qos class and service id to the path record
Yevgeny Kliteynik
kliteyn at dev.mellanox.co.il
Mon Jan 22 23:07:34 PST 2007
Sasha Khapyorsky wrote:
> Hi Yevgeny,
>
> On 18:05 Mon 22 Jan , Yevgeny Kliteynik wrote:
>> Hi Hal
>>
>> QoS patch: added qos class and service id to the path record
>>
>> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
>
> Couple of comments...
>
>> ---
>> osm/include/iba/ib_types.h | 149 +++++++++++++++++++++++++++++++---
>> osm/opensm/osm_helper.c | 8 +-
>> osm/opensm/osm_sa_multipath_record.c | 2 +-
>> osm/opensm/osm_sa_path_record.c | 5 +-
>> osm/osmtest/osmtest.c | 2 +-
>> 5 files changed, 147 insertions(+), 19 deletions(-)
>>
>> diff --git a/osm/include/iba/ib_types.h b/osm/include/iba/ib_types.h
>> index 22f7f62..7762ed2 100644
>> --- a/osm/include/iba/ib_types.h
>> +++ b/osm/include/iba/ib_types.h
>> @@ -1700,6 +1700,28 @@ ib_class_is_rmpp(
>> #define IB_SMINFO_STATE_MASTER 3
>> /**********/
>>
>> +/****d* IBA Base: Constants/IB_PATH_REC_SL_MASK
>> +* NAME
>> +* IB_PATH_REC_SL_MASK
>> +*
>> +* DESCRIPTION
>> +* Mask for the sl field for path record
>> +*
>> +* SOURCE
>> +*/
>> +#define IB_PATH_REC_SL_MASK 0xF
>> +
>> +/****d* IBA Base: Constants/IB_PATH_REC_QOS_CLASS_MASK
>> +* NAME
>> +* IB_PATH_REC_QOS_CLASS_MASK
>> +*
>> +* DESCRIPTION
>> +* Mask for the QoS class field for path record
>> +*
>> +* SOURCE
>> +*/
>> +#define IB_PATH_REC_QOS_CLASS_MASK 0xFFF0
>> +
>> /****d* IBA Base: Constants/IB_PATH_REC_SELECTOR_MASK
>> * NAME
>> * IB_PATH_REC_SELECTOR_MASK
>> @@ -2314,7 +2336,7 @@ ib_gid_get_guid(
>> #include <complib/cl_packon.h>
>> typedef struct _ib_path_rec
>> {
>> - uint8_t resv0[8];
>> + ib_net64_t service_id;
>> ib_gid_t dgid;
>> ib_gid_t sgid;
>> ib_net16_t dlid;
>> @@ -2323,7 +2345,7 @@ typedef struct _ib_path_rec
>> uint8_t tclass;
>> uint8_t num_path;
>> ib_net16_t pkey;
>> - ib_net16_t sl;
>> + ib_net16_t qos_class_sl;
>> uint8_t mtu;
>> uint8_t rate;
>> uint8_t pkt_life;
>> @@ -2363,11 +2385,8 @@ typedef struct _ib_path_rec
>> * pkey
>> * Partition key (P_Key) to use on this path.
>> *
>> -* resv1
>> -* Reserved byte.
>> -*
>> -* 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
>> @@ -2388,6 +2407,7 @@ typedef struct _ib_path_rec
>> *********/
>>
>> /* Path Record Component Masks */
>> +#define IB_PR_COMPMASK_SERVICEID (CL_HTON64(((uint64_t)1)<<1))
>> #define IB_PR_COMPMASK_DGID (CL_HTON64(((uint64_t)1)<<2))
>> #define IB_PR_COMPMASK_SGID (CL_HTON64(((uint64_t)1)<<3))
>> #define IB_PR_COMPMASK_DLID (CL_HTON64(((uint64_t)1)<<4))
>> @@ -2400,7 +2420,7 @@ typedef struct _ib_path_rec
>> #define IB_PR_COMPMASK_REVERSIBLE (CL_HTON64(((uint64_t)1)<<11))
>> #define IB_PR_COMPMASK_NUMBPATH (CL_HTON64(((uint64_t)1)<<12))
>> #define IB_PR_COMPMASK_PKEY (CL_HTON64(((uint64_t)1)<<13))
>> -#define IB_PR_COMPMASK_RESV1 (CL_HTON64(((uint64_t)1)<<14))
>> +#define IB_PR_COMPMASK_QOS_CLASS (CL_HTON64(((uint64_t)1)<<14))
>> #define IB_PR_COMPMASK_SL (CL_HTON64(((uint64_t)1)<<15))
>> #define IB_PR_COMPMASK_MTUSELEC (CL_HTON64(((uint64_t)1)<<16))
>> #define IB_PR_COMPMASK_MTU (CL_HTON64(((uint64_t)1)<<17))
>> @@ -2658,6 +2678,7 @@ ib_path_rec_init_local(
>> IN ib_net16_t slid,
>> IN uint8_t num_path,
>> IN ib_net16_t pkey,
>> + IN uint16_t qos_class,
>> IN uint8_t sl,
>> IN uint8_t mtu_selector,
>> IN uint8_t mtu,
>> @@ -2673,8 +2694,8 @@ ib_path_rec_init_local(
>> p_rec->slid = slid;
>> p_rec->num_path = num_path;
>> p_rec->pkey = pkey;
>> - /* Lower 4 bits of path rec's SL are reserved. */
>> - p_rec->sl = cl_ntoh16( sl );
>> + p_rec->qos_class_sl = cl_hton16( (sl & IB_PATH_REC_SL_MASK) |
>> + (qos_class << 4) );
>> p_rec->mtu = (uint8_t)((mtu & IB_PATH_REC_BASE_MASK) |
>> (uint8_t)(mtu_selector << 6));
>> p_rec->rate = (uint8_t)((rate & IB_PATH_REC_BASE_MASK) |
>> @@ -2686,8 +2707,8 @@ ib_path_rec_init_local(
>> /* Clear global routing fields for local path records */
>> p_rec->hop_flow_raw = 0;
>> p_rec->tclass = 0;
>> + p_rec->service_id = 0;
>>
>> - *((uint64_t*)p_rec->resv0) = 0;
>> *((uint32_t*)p_rec->resv2) = 0;
>> *((uint16_t*)p_rec->resv2 + 2) = 0;
>> }
>> @@ -2716,6 +2737,9 @@ ib_path_rec_init_local(
>> * pkey
>> * [in] Partition key (P_Key) to use on this path.
>> *
>> +* qos_class
>> +* [in] QoS class to use on this path. Lower 12-bits are valid.
>> +*
>> * sl
>> * [in] Service level to use on this path. Lower 4-bits are valid.
>> *
>> @@ -2779,6 +2803,41 @@ ib_path_rec_num_path(
>> * ib_path_rec_t
>> *********/
>>
>> +/****f* IBA Base: Types/ib_path_rec_set_sl
>> +* NAME
>> +* ib_path_rec_set_sl
>> +*
>> +* DESCRIPTION
>> +* Set path service level.
>> +*
>> +* SYNOPSIS
>> +*/
>> +static inline void OSM_API
>> +ib_path_rec_set_sl(
>> + IN ib_path_rec_t* const p_rec,
>> + IN const uint8_t sl )
>> +{
>> + p_rec->qos_class_sl = cl_hton16( ( cl_ntoh16(p_rec->qos_class_sl) &
>> + IB_PATH_REC_QOS_CLASS_MASK ) |
>> + ( sl & IB_PATH_REC_SL_MASK) );
>> +}
>> +/*
>> +* PARAMETERS
>> +* p_rec
>> +* [in] Pointer to the path record object.
>> +*
>> +* sl
>> +* [in] Service level to set.
>> +*
>> +* RETURN VALUES
>> +* None
>> +*
>> +* NOTES
>> +*
>> +* SEE ALSO
>> +* ib_path_rec_t
>> +*********/
>> +
>> /****f* IBA Base: Types/ib_path_rec_sl
>> * NAME
>> * ib_path_rec_sl
>> @@ -2792,7 +2851,7 @@ static inline uint8_t OSM_API
>> ib_path_rec_sl(
>> IN const ib_path_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_PATH_REC_SL_MASK) );
>> }
>> /*
>> * PARAMETERS
>> @@ -2808,6 +2867,72 @@ ib_path_rec_sl(
>> * ib_path_rec_t
>> *********/
>>
>> +/****f* IBA Base: Types/ib_path_rec_set_qos_class
>> +* NAME
>> +* ib_path_rec_set_qos_class
>> +*
>> +* DESCRIPTION
>> +* Set path QoS class.
>> +*
>> +* SYNOPSIS
>> +*/
>> +static inline void OSM_API
>> +ib_path_rec_set_qos_class(
>> + IN ib_path_rec_t* const p_rec,
>> + IN const uint16_t qos_class )
>> +{
>> + p_rec->qos_class_sl = cl_hton16( ( cl_ntoh16(p_rec->qos_class_sl) &
>> + IB_PATH_REC_QOS_CLASS_MASK ) |
>> + ( qos_class << 4) );
>> +}
>
> IB_PATH_REC_QOS_CLASS_MASK is 0xfff0, so this will clear sl component.
Right, this is a bug. Good thing you spotted it.
It should be IB_PATH_REC_SL_MASK instead of IB_PATH_REC_QOS_CLASS_MASK.
>> +/*
>> +* PARAMETERS
>> +* p_rec
>> +* [in] Pointer to the path record object.
>> +*
>> +* qos_class
>> +* [in] QoS class to set.
>> +*
>> +* RETURN VALUES
>> +* None
>> +*
>> +* NOTES
>> +*
>> +* SEE ALSO
>> +* ib_path_rec_t
>> +*********/
>> +
>> +/****f* IBA Base: Types/ib_path_rec_qos_class
>> +* NAME
>> +* ib_path_rec_qos_class
>> +*
>> +* DESCRIPTION
>> +* Get QoS class.
>> +*
>> +* SYNOPSIS
>> +*/
>> +static inline uint16_t OSM_API
>> +ib_path_rec_qos_class(
>> + IN const ib_path_rec_t* const p_rec )
>> +{
>> + return( (uint16_t)( cl_ntoh16( p_rec->qos_class_sl ) &
>
> Why (uint16_t) casting is needed?
>
>> + IB_PATH_REC_QOS_CLASS_MASK ) >> 4 );
>> +}
>
> &IB_PATH_REC_QOS_CLASS_MASK is not needed - follow >> 4 drops lower bits.
Right again (though not a bug this time).
Instead of this:
return( (uint16_t)( cl_ntoh16( p_rec->qos_class_sl ) &
IB_PATH_REC_QOS_CLASS_MASK ) >> 4 );
there can be simply this:
return( cl_ntoh16( p_rec->qos_class_sl ) >> 4 );
Hal, should I resubmit the patch?
Thanks.
-- Yevgeny
>
> Sasha
>
>> +/*
>> +* PARAMETERS
>> +* p_rec
>> +* [in] Pointer to the path record object.
>> +*
>> +* RETURN VALUES
>> +* QoS class of the path record.
>> +*
>> +* NOTES
>> +*
>> +* SEE ALSO
>> +* ib_path_rec_t
>> +*********/
>> +
>> +
>> /****f* IBA Base: Types/ib_path_rec_mtu
>> * NAME
>> * ib_path_rec_mtu
>> diff --git a/osm/opensm/osm_helper.c b/osm/opensm/osm_helper.c
>> index 2ef8e38..e0b5aef 100644
>> --- a/osm/opensm/osm_helper.c
>> +++ b/osm/opensm/osm_helper.c
>> @@ -1095,7 +1095,7 @@ osm_dump_path_record(
>> {
>> osm_log( p_log, log_level,
>> "PathRecord dump:\n"
>> - "\t\t\t\tresv0...................0x%016" PRIx64 "\n"
>> + "\t\t\t\tservice_id..............0x%016" PRIx64 "\n"
>> "\t\t\t\tdgid....................0x%016" PRIx64 " : "
>> "0x%016" PRIx64 "\n"
>> "\t\t\t\tsgid....................0x%016" PRIx64 " : "
>> @@ -1106,6 +1106,7 @@ osm_dump_path_record(
>> "\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\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"
>> @@ -1114,7 +1115,7 @@ osm_dump_path_record(
>> "\t\t\t\tresv2...................0x%X\n"
>> "\t\t\t\tresv3...................0x%X\n"
>> "",
>> - *(uint64_t*)p_pr->resv0,
>> + cl_ntoh64(p_pr->service_id),
>> cl_ntoh64( p_pr->dgid.unicast.prefix ),
>> cl_ntoh64( p_pr->dgid.unicast.interface_id ),
>> cl_ntoh64( p_pr->sgid.unicast.prefix ),
>> @@ -1125,7 +1126,8 @@ osm_dump_path_record(
>> p_pr->tclass,
>> p_pr->num_path,
>> cl_ntoh16( p_pr->pkey ),
>> - cl_ntoh16( p_pr->sl ),
>> + ib_path_rec_qos_class(p_pr),
>> + ib_path_rec_sl(p_pr),
>> p_pr->mtu,
>> p_pr->rate,
>> p_pr->pkt_life,
>> diff --git a/osm/opensm/osm_sa_multipath_record.c b/osm/opensm/osm_sa_multipath_record.c
>> index 2f61fb8..5ec0006 100644
>> --- a/osm/opensm/osm_sa_multipath_record.c
>> +++ b/osm/opensm/osm_sa_multipath_record.c
>> @@ -759,7 +759,7 @@ __osm_mpr_rcv_build_pr(
>> p_pr->hop_flow_raw &= cl_hton32(1<<31);
>>
>> p_pr->pkey = p_parms->pkey;
>> - p_pr->sl = cl_hton16( p_parms->sl );
>> + ib_path_rec_set_sl(p_pr, p_parms->sl);
>> p_pr->mtu = (uint8_t)( p_parms->mtu | 0x80 );
>> p_pr->rate = (uint8_t)( p_parms->rate | 0x80 );
>>
>> diff --git a/osm/opensm/osm_sa_path_record.c b/osm/opensm/osm_sa_path_record.c
>> index 7707f52..5a43912 100644
>> --- a/osm/opensm/osm_sa_path_record.c
>> +++ b/osm/opensm/osm_sa_path_record.c
>> @@ -774,7 +774,8 @@ __osm_pr_rcv_build_pr(
>> #endif
>>
>> p_pr->pkey = p_parms->pkey;
>> - p_pr->sl = cl_hton16(p_parms->sl);
>> + ib_path_rec_set_qos_class(p_pr,0);
>> + ib_path_rec_set_sl(p_pr,p_parms->sl);
>> p_pr->mtu = (uint8_t)(p_parms->mtu | 0x80);
>> p_pr->rate = (uint8_t)(p_parms->rate | 0x80);
>>
>> @@ -2051,7 +2052,7 @@ osm_pr_rcv_process(
>> /* SL, Hop Limit, and Flow Label */
>> ib_member_get_sl_flow_hop( p_mgrp->mcmember_rec.sl_flow_hop,
>> &sl, &flow_label, &hop_limit );
>> - p_pr_item->path_rec.sl = cl_hton16( sl );
>> + ib_path_rec_set_sl(&(p_pr_item->path_rec), sl);
>> #ifndef ROUTER_EXP
>> p_pr_item->path_rec.hop_flow_raw = cl_hton32(hop_limit) |
>> (flow_label << 8);
>> diff --git a/osm/osmtest/osmtest.c b/osm/osmtest/osmtest.c
>> index b9e3bf7..c42b037 100644
>> --- a/osm/osmtest/osmtest.c
>> +++ b/osm/osmtest/osmtest.c
>> @@ -1982,7 +1982,7 @@ osmtest_write_path_info( IN osmtest_t *
>> cl_ntoh64( p_rec->sgid.unicast.interface_id ),
>> cl_ntoh16( p_rec->dlid ), cl_ntoh16( p_rec->slid ),
>> cl_ntoh32( p_rec->hop_flow_raw ), p_rec->tclass,
>> - p_rec->num_path, cl_ntoh16( p_rec->pkey ), p_rec->sl,
>> + p_rec->num_path, cl_ntoh16( p_rec->pkey ), ib_path_rec_sl(p_rec),
>> p_rec->mtu, p_rec->rate, p_rec->pkt_life,
>> p_rec->preference );
>>
>> --
>> 1.4.4.1.GIT
>>
>>
>>
>> _______________________________________________
>> openib-general mailing list
>> openib-general at openib.org
>> http://openib.org/mailman/listinfo/openib-general
>>
>> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>>
>
More information about the general
mailing list