>
> On Fri, Apr 20, 2018 at 01:32:32PM -0500, Steve Wise wrote:
> > > > +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.
>
> Oh gross.. no..
>
> > > > + 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.
>
> Also gross, no..
>
Any suggestion on how the rdma tool should display this opaque data so it is
readable?
> The kernel shouldn't be involved in formatting like this.
>
> > > > + 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.
>
> I don't know what this means
>
> 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
