>
>
> > -----Original Message-----
> > From: linux-rdma-owner@xxxxxxxxxxxxxxx <linux-rdma-
> > owner@xxxxxxxxxxxxxxx> On Behalf Of Leon Romanovsky
> > Sent: Friday, March 30, 2018 5:36 AM
> > To: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> > Cc: jgg@xxxxxxxxxxxx; dledford@xxxxxxxxxx; linux-
> rdma@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH v2 rdma-next 3/5] RDMA/nldev: add provider-specific
> > device/port tracking
> >
> > On Thu, Mar 29, 2018 at 09:09:51AM -0700, Steve Wise wrote:
> > > Add fill_dev_info and fill_port_info functions to rdma_restrack_root.
> > > This allows providers to have provider-specific fill functions for
device
> > > and port restrack operations.
> > >
> > > Signed-off-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> > > ---
> > > drivers/infiniband/core/nldev.c | 35 +++++++++++++++++++++++++++++-
> --
> > ---
> > > include/rdma/restrack.h | 15 +++++++++++++++
> > > 2 files changed, 44 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/nldev.c
> > b/drivers/infiniband/core/nldev.c
> > > index 99df8d4..902517f 100644
> > > --- a/drivers/infiniband/core/nldev.c
> > > +++ b/drivers/infiniband/core/nldev.c
> > > @@ -115,6 +115,22 @@ static int provider_fill_res_entry(struct
> > rdma_restrack_root *resroot,
> > > resroot->fill_res_entry(msg, cb, res) : 0;
> > > }
> > >
> > > +static int provider_fill_dev_info(struct sk_buff *msg,
> > > + struct netlink_callback *cb,
> > > + struct ib_device *device)
> > > +{
> > > + return device->res.fill_dev_info ?
> > > + device->res.fill_dev_info(msg, cb, device) : 0;
> > > +}
> > > +
> > > +static int provider_fill_port_info(struct sk_buff *msg,
> > > + struct netlink_callback *cb,
> > > + struct ib_device *device, u32 port)
> > > +{
> > > + return device->res.fill_port_info ?
> > > + device->res.fill_port_info(msg, cb, device, port) : 0;
> > > +}
> > > +
> > > static int fill_nldev_handle(struct sk_buff *msg, struct ib_device
> *device)
> > > {
> > > if (nla_put_u32(msg, RDMA_NLDEV_ATTR_DEV_INDEX, device-
> > >index))
> > > @@ -125,7 +141,8 @@ static int fill_nldev_handle(struct sk_buff *msg,
> > struct ib_device *device)
> > > return 0;
> > > }
> > >
> > > -static int fill_dev_info(struct sk_buff *msg, struct ib_device
*device)
> > > +static int fill_dev_info(struct sk_buff *msg, struct netlink_callback
*cb,
> > > + struct ib_device *device)
> > > {
> > > char fw[IB_FW_VERSION_NAME_MAX];
> > >
> > > @@ -156,10 +173,14 @@ static int fill_dev_info(struct sk_buff *msg,
> > struct ib_device *device)
> > > return -EMSGSIZE;
> > > if (nla_put_u8(msg, RDMA_NLDEV_ATTR_DEV_NODE_TYPE, device-
> > >node_type))
> > > return -EMSGSIZE;
> > > +
> > > + if (provider_fill_dev_info(msg, cb, device))
> > > + return -EMSGSIZE;
> > > +
> > > return 0;
> > > }
> > >
> > > -static int fill_port_info(struct sk_buff *msg,
> > > +static int fill_port_info(struct sk_buff *msg, struct
netlink_callback *cb,
> > > struct ib_device *device, u32 port)
> > > {
> > > struct ib_port_attr attr;
> > > @@ -195,6 +216,8 @@ static int fill_port_info(struct sk_buff *msg,
> > > return -EMSGSIZE;
> > > if (nla_put_u8(msg, RDMA_NLDEV_ATTR_PORT_PHYS_STATE,
> > attr.phys_state))
> > > return -EMSGSIZE;
> > > + if (provider_fill_port_info(msg, cb, device, port))
> > > + return -EMSGSIZE;
> > > return 0;
> > > }
> > >
> > > @@ -554,7 +577,7 @@ static int nldev_get_doit(struct sk_buff *skb,
> > struct nlmsghdr *nlh,
> > > RDMA_NL_GET_TYPE(RDMA_NL_NLDEV,
> > RDMA_NLDEV_CMD_GET),
> > > 0, 0);
> > >
> > > - err = fill_dev_info(msg, device);
> > > + err = fill_dev_info(msg, NULL, device);
> >
> > I look on this line and think that providing "struct netlink_callback
*cb"
> > to drivers is too broad approach. The right thing will be to pass
> > "u64 caps_falgs" variable which will have bits on/off per
> > uapi/linux/capability.h.
> >
> > However, you don't use this functionality for now, so it will make sense
> > do not implement it completely and simply leave comment in the code
> how
> > it should be implemented in the future.
> >
> > Thanks
>
> Do you think the fill_res_entry API should also have this change? Is
there
> still time for me to respin this and make the merge window?
>
Hmm, Look at __netlink_ns_capable(). I'm not sure how to compose the
correct cap_flags, and there is no existing API to return them.
--
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
