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

[Sagi Grimberg]

On 8/31/22 22:38, 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.
> 
> This is needed to diagnose and identify connectivity issues since it will
> allow 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));
> +

For every queue?

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

You can just retrieve the sock_addr from the admin queue here locally. I
don't think we need a struct member for it.

> +
> +	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);

It is confusing to expose that if it was not specified, that is creating
a different behavior from the existing behavior and from other
transports.

> +	} 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;
> +}
> +
>   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,
>   };
>