Re: [PATCH 08/11] IB/cma: Add net_dev and private data checks to RDMA CM

Jason Gunthorpe
2015-06-17T17:18:43Z

On Tue, Jun 16, 2015 at 08:26:26AM +0300, Haggai Eran wrote:
> On 15/06/2015 20:08, Jason Gunthorpe wrote:
> > On Mon, Jun 15, 2015 at 11:47:13AM +0300, Haggai Eran wrote:
> >> Instead of relying on a the ib_cm module to check an incoming CM request's
> >> private data header, add these checks to the RDMA CM module. This allows a
> >> following patch to to clean up the ib_cm interface and remove the code that
> >> looks into the private headers. It will also allow supporting namespaces in
> >> RDMA CM by making these checks namespace aware later on.
> >
> > I was expecting one of these patches to flow the net_device from here:
> >
> >> +static struct net_device *cma_get_net_dev(struct ib_cm_event *ib_event,
> >> + const struct cma_req_info *req)
> >> +{
> >
> > Down through cma_req_handler and cma_new_conn_id so that we get rid of
> > the cma_translate_addr on the ingress side.
> >
> > Having the ingress side use one ingress net_device for all processing
> > seems very important to me...
>
> Is it really very important? I thought the bound_dev_if of a passive
> connection id is only used by the netlink statistics mechanism.

I mean 'very important' in the sense it makes the RDMA-CM *make
logical sense*, not so much in the 'can user space tell'.

So yes, cleaning this seems very important to establish that logical
narrative of how the packet flows through this code.

Plus, there is an init_net in the cma_translate_addr path that needs to
be addressed - so purging cma_translate_addr is a great way to handle
that. That would leave only the call in rdma_bind_addr, and for bind,
the process net namespace is the correct thing to use.

Jason