[PATCH] nvme-tcp: Print actual source IP address in address attribute

[Chaitanya Kulkarni]

On 8/31/22 11:50, Martin Belanger wrote:
> From: Martin Belanger <martin.belanger at dell.com>
> 
> TCP transport relies on the routing table to determine which
> source address and interface to use when making a connection.
> Currently, there is no way to tell from userspace where a
> connection was made. This patch allows exposing the actual source
> address using the host_traddr field of the address attribute.
> 
> Why is this important? This will make it possible to diagnose and
> identify connectivity issues. With this, we'll be able to tell
> which interface and source address is associated with each
> connection.
> 

Perhaps remove question in the commit log and rephrase it :-

This is needed to diagnose and identify connectivity issues since
it allows us to tell which interface and source address is associated
with each connection.

> Signed-off-by: Martin Belanger <martin.belanger at dell.com>
> ---
>   drivers/nvme/host/tcp.c | 34 +++++++++++++++++++++++++++++++++-
>   1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 044da18c06f5..adbb76483a74 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -165,6 +165,7 @@ struct nvme_tcp_ctrl {
>   	struct blk_mq_tag_set	admin_tag_set;
>   	struct sockaddr_storage addr;
>   	struct sockaddr_storage src_addr;
> +	struct sockaddr_storage sock_addr;
>   	struct nvme_ctrl	ctrl;
>   
>   	struct work_struct	err_work;
> @@ -1597,6 +1598,12 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl,
>   		goto err_rcv_pdu;
>   	}
>   
> +	/* Retrieve the actual source address from the socket */
> +	ret = kernel_getsockname(queue->sock,
> +		(struct sockaddr *)&ctrl->sock_addr);
> +	if (ret < 0)
> +		memset(&ctrl->sock_addr, 0, sizeof(ctrl->sock_addr));
> +

without looking into the detailed code, quick question is there a hard
requirement that we need to do above work in  nvme_tcp_alloc_queue()
before we call nvme_tcp_init_connection() ?

If not then can we move above code to the nvme_tcp_get_address() to
avoid adding a new member nvme_tcp_ctrl structure ?

>   	ret = nvme_tcp_init_connection(queue);
>   	if (ret)
>   		goto err_init_connect;
> @@ -2532,6 +2539,31 @@ static int nvme_tcp_poll(struct blk_mq_hw_ctx *hctx, struct io_comp_batch *iob)
>   	return queue->nr_cqe;
>   }
>   
> +static int nvme_tcp_get_address(struct nvme_ctrl *ctrl, char *buf, int size)
> +{
> +	int len = 0;
> +	struct sockaddr_storage *sa = &to_tcp_ctrl(ctrl)->sock_addr;
> +

nice to have :-

	struct sockaddr_storage *sa = &to_tcp_ctrl(ctrl)->sock_addr;
	int len = 0;

> +	if (ctrl->opts->mask & NVMF_OPT_TRADDR)
> +		len += scnprintf(buf, size, "traddr=%s", ctrl->opts->traddr);
> +	if (ctrl->opts->mask & NVMF_OPT_TRSVCID)
> +		len += scnprintf(buf + len, size - len, "%strsvcid=%s",
> +				(len) ? "," : "", ctrl->opts->trsvcid);
> +	if ((sa->ss_family == AF_INET) || (sa->ss_family == AF_INET6)) {
> +		len += scnprintf(buf + len, size - len, "%shost_traddr=%pISc",
> +				(len) ? "," : "", sa);
> +	} else if (ctrl->opts->mask & NVMF_OPT_HOST_TRADDR) {
> +		len += scnprintf(buf + len, size - len, "%shost_traddr=%s",
> +				(len) ? "," : "", ctrl->opts->host_traddr);
> +	}
> +	if (ctrl->opts->mask & NVMF_OPT_HOST_IFACE)
> +		len += scnprintf(buf + len, size - len, "%shost_iface=%s",
> +				(len) ? "," : "", ctrl->opts->host_iface);
> +	len += scnprintf(buf + len, size - len, "\n");
> +
> +	return len;

nit:- how about following ?

return len + scnprintf(buf + len, size - len, "\n");

> +}
> +
>   static const struct blk_mq_ops nvme_tcp_mq_ops = {
>   	.queue_rq	= nvme_tcp_queue_rq,
>   	.commit_rqs	= nvme_tcp_commit_rqs,
> @@ -2563,7 +2595,7 @@ static const struct nvme_ctrl_ops nvme_tcp_ctrl_ops = {
>   	.free_ctrl		= nvme_tcp_free_ctrl,
>   	.submit_async_event	= nvme_tcp_submit_async_event,
>   	.delete_ctrl		= nvme_tcp_delete_ctrl,
> -	.get_address		= nvmf_get_address,
> +	.get_address		= nvme_tcp_get_address,
>   	.stop_ctrl		= nvme_tcp_stop_ctrl,
>   };
>