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 <[EMAIL PROTECTED]>

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.

> +/*
> +* 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.

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
> [email protected]
> http://openib.org/mailman/listinfo/openib-general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
> 

_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to