On Tue, Jun 09, 2020 at 11:27:08AM +0300, Maor Gottlieb wrote: > > On 6/8/2020 2:46 PM, Jason Gunthorpe wrote: > > On Sun, Jun 07, 2020 at 11:47:11AM +0300, Maor Gottlieb wrote: > > > On 6/2/2020 4:23 PM, Leon Romanovsky wrote: > > > > On Tue, Jun 02, 2020 at 09:27:02AM -0300, Jason Gunthorpe wrote: > > > > > On Tue, Jun 02, 2020 at 09:21:18AM +0300, Leon Romanovsky wrote: > > > > > > On Mon, Jun 01, 2020 at 09:26:46AM -0300, Jason Gunthorpe wrote: > > > > > > > On Sun, May 31, 2020 at 12:54:14PM +0300, Leon Romanovsky wrote: > > > > > > > > On Fri, May 29, 2020 at 08:31:21PM -0300, Jason Gunthorpe wrote: > > > > > > > > > On Wed, May 27, 2020 at 04:54:08PM +0300, Leon Romanovsky wrote: > > > > > > > > > > From: Maor Gottlieb <maorg@xxxxxxxxxxxx> > > > > > > > > > > > > > > > > > > > > Add support to get MR (mkey) resource dump in RAW format. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Maor Gottlieb <maorg@xxxxxxxxxxxx> > > > > > > > > > > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > > > > > > > > > drivers/infiniband/hw/mlx5/restrack.c | 3 ++- > > > > > > > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/infiniband/hw/mlx5/restrack.c b/drivers/infiniband/hw/mlx5/restrack.c > > > > > > > > > > index 9e1389b8dd9f..834886536127 100644 > > > > > > > > > > +++ b/drivers/infiniband/hw/mlx5/restrack.c > > > > > > > > > > @@ -116,7 +116,8 @@ int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg, > > > > > > > > > > struct nlattr *table_attr; > > > > > > > > > > > > > > > > > > > > if (raw) > > > > > > > > > > - return -EOPNOTSUPP; > > > > > > > > > > + return fill_res_raw(msg, mr->dev, MLX5_SGMT_TYPE_PRM_QUERY_MKEY, > > > > > > > > > > + mlx5_mkey_to_idx(mr->mmkey.key)); > > > > > > > > > None of the raw functions actually share any code with the non raw > > > > > > > > > part, why are the in the same function? In fact all the implemenations > > > > > > > > > just call some other function for raw. > > > > > > > > > > > > > > > > > > To me this looks like they should should all be a new op > > > > > > > > > 'fill_raw_res_mr_entry' and drop the 'bool' > > > > > > > > I don't think that this is right approach, we already created ops per-objects > > > > > > > > o remove API multiplexing. Extra de-duplication will create too much ops > > > > > > > > without any real benefit. > > > > > > > If there is no code sharing then they should not be in the same > > > > > > > function at all. More ops is not really a problem. > > > > > > Logically they are the same, user asks to get object property, driver returns. > > > > > I'm starting to think it is also a mistake to have the same netlink op > > > > > and trigger it by an inbound attribute. Are there other examples of > > > > > that in netlink? Feels wrong > > > > I have no idea, don't see it in devlink.c > > > Jason, do you mean trigger the raw by inbound attribute? I don't see a > > > reason why not do that. Netlink attributes used for input and > > > output. > > What examples do you have where the input attribute completely changes > > the output format? > > > > Jason > > I don't have. Anyway I am planning to send v2 with new netlink ops. Well, I think you need to look at the netlink interface side too. Jason
