[PATCH 3/3] nvmet: include all configured ports in the discovery log page

[Sagi Grimberg]

> When a per-port discovery subsystem is used we should include
> all configured ports in the discovery log page, not just that one
> through which the controller was connected.
> 
> - /
>    o- ports
>    | o- 1 .. [trtype=tcp, traddr=127.0.0.1, trsvcid=8009]
>    | | o- subsystems
>    | |   o- nqn.discovery
>    | o- 2 .. [trtype=tcp, traddr=127.0.0.1, trsvcid=4420]
>    | | o- subsystems
>    | |   o- nqn.discovery
>    | |   o- nqn.io-1
>    | o- 3 .. [trtype=tcp, traddr=127.0.0.1, trsvcid=4421]
>    |   o- subsystems
>    |     o- nqn.io-2
>    o- subsystems
>      o- nqn.discovery
>      o- nqn.io-1
>      | o- namespaces
>      o- nqn.io-2
>        o- namespaces
> 
> A discovery on port 8009 will return information about
> - nqn.discovery at port 8009
> - nqn.discovery at port 4420
> - nqn.io-1 at port 4420
> A discovery on port 4420 will return the same information.
> A discovery on port 4421 will return information about
> - standard discovery subsystem on port 4421
> - nqn.io-2 on port 4421

This is a different functionality than supporting unique discovery
NQNs.

If I want in your example to use a unique discovery subsystem but to
report only on the port local I have to use two different unique
discovery subsystems. Which is different than the standard discovery
subsystem, why?

I'd submit to you that IMO unique discovery subsystems should not behave
differently than the standard discovery subsystem. Please explain why
do you think that unique discovery subsystems should behave differently
with respect to the content of the log page.

> 
> Signed-off-by: Hannes Reinecke <hare at suse.de>
> ---
>   drivers/nvme/target/discovery.c | 53 +++++++++++++++++++++++++++------
>   1 file changed, 44 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvme/target/discovery.c b/drivers/nvme/target/discovery.c
> index 6b8aa6c4e752..ea8fce538342 100644
> --- a/drivers/nvme/target/discovery.c
> +++ b/drivers/nvme/target/discovery.c
> @@ -149,6 +149,30 @@ static void nvmet_set_disc_traddr(struct nvmet_req *req, struct nvmet_port *port
>   		memcpy(traddr, port->disc_addr.traddr, NVMF_TRADDR_SIZE);
>   }
>   
> +/*
> + * discovery_port_match - filter eligible ports for discovery log page
> + *
> + * If the port through which the controller is connected has no discovery
> + * subsystem linked, use the original behaviour of just including information
> + * about that port in the discovery log page.
> + * Otherwise include information about all ports into which the specified
> + * discovery subsystem is linked.
> + */
> +
> +static bool discovery_port_match(struct nvmet_req *req, struct nvmet_port *r)
> +{
> +	struct nvmet_ctrl *ctrl = req->sq->ctrl;
> +
> +	if (!req->port->disc_subsys) {
> +		if (r != req->port)
> +			return false;
> +	} else {
> +		if (ctrl->subsys != r->disc_subsys)
> +			return false;
> +	}
> +	return true;
> +}
> +
>   static size_t discovery_log_entries(struct nvmet_req *req)
>   {
>   	struct nvmet_ctrl *ctrl = req->sq->ctrl;
> @@ -159,10 +183,14 @@ static size_t discovery_log_entries(struct nvmet_req *req)
>   	if (!req->port->disc_subsys)
>   		entries++;
>   
> -	list_for_each_entry(p, &req->port->subsystems, entry) {
> -		if (!nvmet_host_allowed(p->subsys, ctrl->hostnqn))
> +	list_for_each_entry(r, nvmet_ports, global_entry) {
> +		if (!discovery_port_match(req, r))
>   			continue;
> -		entries++;
> +		list_for_each_entry(p, &r->subsystems, entry) {
> +			if (!nvmet_host_allowed(p->subsys, ctrl->hostnqn))
> +				continue;
> +			entries++;
> +		}
>   	}
>   	list_for_each_entry(r, &req->port->referrals, entry)
>   		entries++;
> @@ -226,14 +254,21 @@ static void nvmet_execute_disc_get_log_page(struct nvmet_req *req)
>   		numrec++;
>   	}
>   
> -	list_for_each_entry(p, &req->port->subsystems, entry) {
> -		if (!nvmet_host_allowed(p->subsys, ctrl->hostnqn))
> +	list_for_each_entry(r, nvmet_ports, global_entry) {
> +		nvmet_set_disc_traddr(req, r, traddr);
> +
> +		if (!discovery_port_match(req, r))
>   			continue;
>   
> -		nvmet_format_discovery_entry(hdr, req->port,
> -				p->subsys->subsysnqn, traddr,
> -				p->subsys->type, numrec);
> -		numrec++;
> +		list_for_each_entry(p, &r->subsystems, entry) {
> +			if (!nvmet_host_allowed(p->subsys, ctrl->hostnqn))
> +				continue;
> +
> +			nvmet_format_discovery_entry(hdr, r,
> +					p->subsys->subsysnqn, traddr,
> +					p->subsys->type, numrec);
> +			numrec++;
> +		}
>   	}
>   
>   	list_for_each_entry(r, &req->port->referrals, entry) {