Re: [PATCH, resend 4/4] IB/srp: Add RDMA/CM support

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3237 bytes --]

On Fri, 2018-01-05 at 10:34 -0700, Jason Gunthorpe wrote:
> On Fri, Jan 05, 2018 at 12:21:10PM -0500, Doug Ledford wrote:
> 
> > > +static int srp_parse_in(struct sockaddr_storage *sa, const char *addr_port_str)
> > > +{
> > > +	char *addr = kstrdup(addr_port_str, GFP_KERNEL);
> > > +	char *port_str = addr;
> > > +	int ret;
> > > +
> > > +	if (!addr)
> > > +		return -ENOMEM;
> > > +	strsep(&port_str, ":");
> > > +	ret = inet_pton_with_scope(&init_net, AF_UNSPEC, addr, port_str, sa);
> > > +	kfree(addr);
> > > +	return ret;
> > > +}
> > > +
> > 
> > This particular function is problematic in that it adds new namespace
> > unaware code.  The namespace code in the RDMA stack is in a limbo state
> > of partially implemented, partially not.  I'm loathe to add any more
> > code that is not fully namespace aware as that just worsens the
> > hysteresis in the stack.  So we need to figure out how to do this in a
> > namespace aware manner.  I haven't previously been thinking about this
> > specific namespace issue, so I'm not prepared to even make suggestions
> > for a fix for this yet...
> 
> Do the userspace daemon's still manage the connection to SRP?
> 
> If yes, then the networking information should be relative to the
> namespace of the thing that wrote to the sysfs file..

Maybe, maybe not.  It depends on the implementation.  IIRC you get one
daemon per port, not one daemon per mount.  In that situation, it's
conceivable that you might want to allow the init_net namespace to host
a master daemon that mounts all mounts on a specific port under a
variety of namespaces and then you don't need to run a separate daemon
in the namespace itself.  You can reduce the privilege level of
applications in the namespace this way.  But, that's an implementation
detail.

> Also, are there srp_daemon patches for this too? I've been asking to
> see the userspace side for new uAPI features before accepting the
> kernel part so that everything can be well understood. If yes, please
> send, even if it is RFCish..

No, the target code is likewise clueless about namespaces.  I suspect
(although I haven't looked) that the target code's Add RDMACM support
patch will have the same problem.

And, FWIW, the iser/isert code also has this same namespace issue.

And this is all complicated by the fact that the block devices created
by the SRP code might be mounted filesystems, or might be presented as
raw block devices to apps.  If it's a filesystem, then we get a pass as
the filesystem layer will determine if any given read/write should go
through based upon the bind mounts of the filesystem and the namespace
of the application accessing the filesystem.  But if the block device is
directly accessed by a user space application, like maybe a database
writing directly to the block device or even just mkfs/fsck, then we
need to enforce namespaces ourselves.  For that we need to know what
namespace(s) the device is created in, and what namespace the app
opening the device is in.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]