Re: [PATCH for-next V1 5/9] IB/core: Add rdma_network_type to wc

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Wed, Nov 25, 2015 at 04:18:25PM +0200, Matan Barak wrote:
> On Wed, Nov 25, 2015 at 8:55 AM, Jason Gunthorpe
> <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> > On Tue, Nov 24, 2015 at 09:07:41PM +0200, Matan Barak wrote:
> >
> >> IMHO, the user is entitles to choose any valid sgid_index for the
> >> interface. Anything he chooses guaranteed to be valid (from security
> >> perspective)
> >
> > No, the namespace patches will have to limit the sgid_indexes that can
> > be used with a QP to those that fall within the namespace. This is
> > another reason I don't like this approach for the kapi.
> 
> By saying namespace, do you mean net namespaces?

Whatever it turns out to be, Haggie was talking about rdma namespaces
for some for this stuff too, but IMHO, rocev2 is pretty clearly
covered under net namespaces.

> If so, the gid cache allows to search by net device (and there's a
> "custom" search that the user can define a filter function which can
> filter by net).
> Anyway, I don't think this cache should be used other than a simple database.

It has nothing to do with the cache, it is everywhere else, you can't
create a qp with a sgid index that is not part of your namespace, for
instance, or recieve a packet on a QP outside your namespace,
etc. Lots of details.



> >> Why do we need to block users who use ibv_rc_pingpong and chose the
> >> GID index correctly by hand?
> >
> > I'm not really concerned with user space, we are stuck with exporting
> > the gid index there.
> 
> So why do we need to block kernel applications from doing the same
> things user-space application can do?

As I explained, it is never correct to use a naked sgid_index and
roceve2, uverbs can't be fixed without a uapi change, but the kernel
can be.

> If there are kernel consumers that want to work with verbs directly,
> they should use ib_init_ah_from_wc and ib_resolve_eth_dmac (or we
> can

As I already said these functions are wrong, they don't have the
routing lookup needed for rocev2. That is my whole point, the
functions that are using the gid cache for rocev2 are *not correct*

I don't really care how you fix it, but every rocev2 sgid-index lookup
in the kernel must be accompanied by a route lookup.

I think the gid cache API design is wrong here because it doesn't
force the above, but whatever, if you choose a different API it
becomes your job to review every patch from now own to make sure other
people use your dangerous API properly.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html