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: 2165 bytes --]

On Thu, 2018-01-04 at 14:28 -0800, Bart Van Assche wrote:
> Since the SRP_LOGIN_REQ defined in the SRP standard is larger than
> what fits in the RDMA/CM login request private data, introduce a new
> login request format for the RDMA/CM.

Ok up to here...

>  Add a kernel module parameter
> to the ib_srp kernel driver

Unless I missed it, there is no new ib_srp kernel module parameter here.
 Furthermore, if there were, I would probably think that was a bad idea.

>  that allows to specify the port on which
> to listen for RDMA/CM connections.

Obviously, a kernel module parameter for the listen port would be an
ib_srpt option, not an ib_srp option, so that makes sense why it's not
in the patch.  The commit log message needs updated.  But, a kernel
module parameter instead of a config file option for a listen port that
we set with the target infrastructure sounds wrong anyway.

>  The default value for this kernel
> module parameter is 0, which means not to listen for RDMA/CM
> connections.

Again, doesn't belong in this commit log.

> 
> +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...

-- 
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 --]