Re: [PATCH] srp_daemon: Use maximum initiator to target IU size

[Bart Van Assche <bvanassche@acm.org>]

On 2019-10-22 00:00, Honggang LI wrote:
> +static bool use_imm_data(void)
> +{
> +#ifdef __linux__
> +	bool ret = false;
> +	char flag = 0;
> +	int cnt;
> +	int fd = open("/sys/module/ib_srp/parameters/use_imm_data", O_RDONLY);
> +
> +	if (fd < 0)
> +		return false;
> +	cnt = read(fd, &flag, 1);
> +	if (cnt != 1)
> +		return false;
> +
> +	if (!strncmp(&flag, "Y", 1))
> +		ret = true;
> +	close(fd);
> +	return ret;
> +#else
> +	return false;
> +#endif
> +}

There is already plenty of Linux-specific code in srp_daemon. The #ifdef
__linux__ / #endif guard does not seem useful to me.

There is a file descriptor leak in the above function, namely if read()
returns another value than 1.

The use_imm_data kernel module parameter was introduced in kernel v5.0
(commit 882981f4a411; "RDMA/srp: Add support for immediate data"). The
max_it_iu_size will be introduced in kernel v5.5 (commit 547ed331bbe8;
"RDMA/srp: Add parse function for maximum initiator to target IU size").

So the above check will help for kernel versions before v5.0 but not for
kernel versions [v5.0..v5.5). If that is really what you want, please
explain this in a comment above the use_imm_data() function.

Thanks,

Bart.