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.
> +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??
> +
> + if (rdma_nl_put_provider_u32(msg, " idx", idx))
Why leading spaces? No.. Saw this a couple of times.
> +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.
Jason
--
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
