On Fri, Jul 27, 2018 at 03:39:32PM +0000, Parav Pandit wrote:
>
>
> > From: Jason Gunthorpe <jgg@xxxxxxxx>
> > Sent: Friday, July 27, 2018 10:19 AM
> > To: Parav Pandit <parav@xxxxxxxxxxxx>
> > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford
> > <dledford@xxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>;
> > Yuval Shaia <yuval.shaia@xxxxxxxxxx>; Leon Romanovsky
> > <leonro@xxxxxxxxxxxx>
> > Subject: Re: [PATCH rdma-next v1] RDMA: Check namespace for user space
> > supplied GID index
> >
> > On Fri, Jul 27, 2018 at 04:53:18AM +0000, Parav Pandit wrote:
> > >
> > >
> > > > From: Jason Gunthorpe <jgg@xxxxxxxx>
> > > > Sent: Thursday, July 26, 2018 5:41 PM
> > > > To: Leon Romanovsky <leon@xxxxxxxxxx>
> > > > Cc: Doug Ledford <dledford@xxxxxxxxxx>; Parav Pandit
> > > > <parav@xxxxxxxxxxxx>; RDMA mailing list
> > > > <linux-rdma@xxxxxxxxxxxxxxx>; Yuval Shaia <yuval.shaia@xxxxxxxxxx>;
> > > > Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > > > Subject: Re: [PATCH rdma-next v1] RDMA: Check namespace for user
> > > > space supplied GID index
> > > >
> > > > On Tue, Jul 24, 2018 at 10:09:27AM +0300, Leon Romanovsky wrote:
> > > > > diff --git a/include/rdma/ib_cache.h b/include/rdma/ib_cache.h
> > > > > index 1108d4220276..511ff6be503a 100644
> > > > > +++ b/include/rdma/ib_cache.h
> > > > > @@ -128,8 +128,23 @@ int ib_get_cached_port_state(struct ib_device
> > > > *device,
> > > > > enum ib_port_state *port_active);
> > > > >
> > > > > bool rdma_is_zero_gid(const union ib_gid *gid); -const struct
> > > > > ib_gid_attr *rdma_get_gid_attr(struct ib_device *device,
> > > > > - u8 port_num, int index);
> > > > > +
> > > > > +const struct ib_gid_attr *_rdma_get_gid_attr(struct ib_device *device,
> > > > > + u8 port_num, int index,
> > > > > + struct net *net);
> > > > > +static inline const struct ib_gid_attr * rdma_get_gid_attr(struct
> > > > > +ib_device *device, u8 port_num, int index) {
> > > > > + return _rdma_get_gid_attr(device, port_num, index, NULL); }
> > > >
> > > > I don't think supporting NULL makes much sense here. ie the ability
> > > > for NULL to search all namespaces is just ugly/potential security hole upon
> > misuse.
> > > >
> > > > It should be init_net and this wrapper shouldn't exist - all callers
> > > > of the API are broken for namespaces and should be fixed. Prefer to
> > > > push the init_net out to them.
> > >
> > > For ib_init_ah_attr_from_wc() for IB link layer, it doesn't make
> > > sense, but in current code flow context it seems fine.
> >
> > The ib_init_ah_attr_from_wc() API is only for generating response packets for
> > MAD's (right?), and it must simply reverse the incoming headers. It shouldn't
> Yes. GID attribute are used till the life of cm_id.
> > really care about namespaces or other things, and if it needs a private interface
> > to the gid cache to do so, then so be it.
> >
> We would end up creating two APIs which does same thing, for example
> in future you can demand rdma_find_gid_by_filter(),
> rdma_find_gid_by_port() to have variant with and without net ns in
> it depending on from where it is called.
No.. the point here is that the need for the NULL is very limited, it
really can only be used by ib_init_ah_attr_from_wc(), so no other API
at all should have this functionality at all.
> I guess I need to wait for Stephen Rothwell to finish smc merge
> conflict merge and once the merged code appears in rdma next tree,
> my patch would make sense to avoid another merge conflict?
Ugh, yes, we can't change smc anymore, this might have to wait for the
next cylce.
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
