>
> On Fri, Mar 30, 2018 at 11:03:46AM -0700, Steve Wise wrote:
> > @@ -645,6 +652,7 @@ void c4iw_register_device(struct work_struct
> *work)
> > dev->ibdev.iwcm->add_ref = c4iw_qp_add_ref;
> > dev->ibdev.iwcm->rem_ref = c4iw_qp_rem_ref;
> > dev->ibdev.iwcm->get_qp = c4iw_get_qp;
> > + dev->ibdev.res.fill_res_entry = fill_res_entry;
>
> Wait you added dev_info and port_info too but not using? Don't add
> stuff until you add the driver side.. So just defer that earlier patch.
Leon had asked for this. But I'll pull it for now.
>
> > +static int fill_sq(struct sk_buff *msg, struct t4_wq *wq)
> > +{
> > + struct nlattr *entry_attr;
> > +
> > + /* WQ+SQ */
> > + entry_attr = nla_nest_start(msg,
> RDMA_NLDEV_ATTR_PROVIDER_ENTRY);
> > + if (!entry_attr)
> > + goto err;
>
> Er, you want to nest ? But how will anything figure out what this
> RDMA_NLDEV_ATTR_PROVIDER_ENTRY is connected to? Seems like
> something
> is missing here.
>
> I think I'd rather see no nesting and have you prefix the strings with
> something to group them??
>
The nesting allowed for the rdma tool to take that as a time for a new line.
So you'd get the qp-global stuff for this qp on one line, then the
sq-specific on the next line, and then rq-specific.
> > +
> > + if (rdma_nl_put_provider_u32(msg, " idx", idx))
>
> Why leading spaces? No.. Saw this a couple of times.
Again, this made for nicer formatting on the display side. It is kind of
tricky to get decent formatting/readability with these generic provider
attributes.
>
> > +static int fill_res_qp_entry(struct sk_buff *msg,
> > + struct rdma_restrack_entry *res)
> > +{
> > + struct ib_qp *ibqp = container_of(res, struct ib_qp, res);
> > + struct t4_swsqe *fsp = NULL, *lsp = NULL;
> > + struct t4_swrqe *frp = NULL, *lrp = NULL;
> > + struct c4iw_qp *qhp = to_c4iw_qp(ibqp);
> > + struct t4_swsqe first_sqe, last_sqe;
> > + struct t4_swrqe first_rqe, last_rqe;
> > + u16 first_sq_idx, last_sq_idx;
> > + u16 first_rq_idx, last_rq_idx;
> > + struct nlattr *table_attr;
> > + struct t4_wq wq;
> > +
> > + /* User qp state is not available, so don't dump user qps */
> > + if (qhp->ucontext)
> > + return 0;
> > +
> > + table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_PROVIDER);
> > + if (!table_attr)
> > + goto err;
>
> I think the core code should setup this nest and handle the closing of
> the nest.. The driver should not be able to stuff arbitary things
> outside its assigned scope.
Leon had some requirement to support provider attributes for shared objects.
So each provider could add its nested attributes.
Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
