RE: [PATCH v4 1/4] IB/netlink: Add defines for local service requests through netlink

Wan, Kaike
2015-06-10T18:31:06Z

> From: Hal Rosenstock [mailto:hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org]
> Sent: Wednesday, June 10, 2015 1:47 PM
>
> On 6/9/2015 10:57 AM, kaike.wan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> > From: Kaike Wan <kaike.wan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> > This patch adds netlink defines for SA client, local service group,
> > local service operations, and related attributes.
> >
> > Signed-off-by: Kaike Wan <kaike.wan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: John Fleck <john.fleck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Reviewed-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> > include/uapi/rdma/rdma_netlink.h | 82
> ++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 82 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/uapi/rdma/rdma_netlink.h
> > b/include/uapi/rdma/rdma_netlink.h
> > index 6e4bb42..341e9be 100644
> > --- a/include/uapi/rdma/rdma_netlink.h
> > +++ b/include/uapi/rdma/rdma_netlink.h
> > @@ -7,12 +7,14 @@ enum {
> > RDMA_NL_RDMA_CM = 1,
> > RDMA_NL_NES,
> > RDMA_NL_C4IW,
> > + RDMA_NL_SA,
> > RDMA_NL_NUM_CLIENTS
> > };
> >
> > enum {
> > RDMA_NL_GROUP_CM = 1,
> > RDMA_NL_GROUP_IWPM,
> > + RDMA_NL_GROUP_LS,
> > RDMA_NL_NUM_GROUPS
> > };
> >
> > @@ -128,5 +130,85 @@ enum {
> > IWPM_NLA_ERR_MAX
> > };
> >
> > +/* Local service group opcodes */
> > +enum {
> > + RDMA_NL_LS_OP_RESOLVE = 0,
> > + RDMA_NL_LS_OP_SET_TIMEOUT,
> > + RDMA_NL_LS_NUM_OPS
> > +};
> > +
> > +/* Local service netlink message flags */
> > +#define RDMA_NL_LS_F_OK 0x0100 /* Success response */
> > +#define RDMA_NL_LS_F_ERR 0x0200 /* Failed response */
> > +
> > +/* Local service attribute type */
> > +enum {
> > + LS_NLA_TYPE_STATUS = 0,
> > + LS_NLA_TYPE_PATH_RECORD,
> > + LS_NLA_TYPE_TIMEOUT,
> > + LS_NLA_TYPE_SERVICE_ID,
> > + LS_NLA_TYPE_DGID,
> > + LS_NLA_TYPE_SGID,
> > + LS_NLA_TYPE_TCLASS,
> > + LS_NLA_TYPE_REVERSIBLE,
> > + LS_NLA_TYPE_NUM_PATH,
>
> Should this be NUMB_PATH rather than NUM_PATH ?

Yes.

>
> > + LS_NLA_TYPE_PKEY,
> > + LS_NLA_TYPE_QOS_CLASS,
>
> Should this include SL too ?

It will be another attribute. All the fields are modeled after struct ib_sa_path_rec and struct ib_user_path_rec, not after struct ibv_path_record in the user space. In addition, only those pathrecord components that are currently used by rdma_cm, ipoib, and srp are list here.

>
> > + LS_NLA_TYPE_MAX
> > +};
> > +
> > +/* Local service status attribute */
> > +enum {
> > + LS_NLA_STATUS_SUCCESS = 0,
> > + LS_NLA_STATUS_EINVAL,
> > + LS_NLA_STATUS_ENODATA,
> > + LS_NLA_STATUS_MAX
> > +};
> > +
> > +struct rdma_nla_ls_status {
> > + __u32 status;
> > +};
> > +
> > +/* Local service pathrecord attribute: struct ib_path_rec_data */
> > +
> > +/* Local service timeout attribute */ struct rdma_nla_ls_timeout {
> > + __u32 timeout;
> > +};
> > +
> > +/* Local Service ServiceID attribute */ struct rdma_nla_ls_service_id
> > +{
> > + __be64 service_id;
> > +};
> > +
> > +/* Local Service DGID/SGID attribute: big endian */ struct
> > +rdma_nla_ls_gid {
> > + __u8 gid[16];
> > +};
> > +
> > +/* Local Service Traffic Class attribute */ struct rdma_nla_ls_tclass
> > +{
> > + __u8 tclass;
> > +};
> > +
> > +/* Local Service Reversible attribute */ struct
> > +rdma_nla_ls_reversible {
> > + __u32 reversible;
> > +};
>
> Isn't __u8 sufficient for reversible ?
Certainly enough. However, reversible is __u32 in struct ib_user_path_rec and int in struct ib_sa_path_rec.

>
> > +
> > +/* Local Service numb_path attribute */ struct rdma_nla_ls_numb_path
> > +{
> > + __u8 numb_path;
> > +};
> > +
> > +/* Local Service Pkey attribute*/
> > +struct rdma_nla_ls_pkey {
> > + __be16 pkey;
> > +};
> > +
> > +/* Local Service Qos Class attribute */ struct rdma_nla_ls_qos_class
> > +{
> > + __be16 qos_class;
> > +};
> >
> > #endif /* _UAPI_RDMA_NETLINK_H */