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

Hannes Reinecke
2022-05-11T05:41:19Z

On 5/10/22 11:43, Sagi Grimberg wrote:
>
>
> On 4/20/22 12:27, Hannes Reinecke wrote:
>> 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
>>
>> Signed-off-by: Hannes Reinecke <hare@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;
>
> What is this condition for? What is this filtering out?

This filters out all ports into which the discovery subsystem
is _not_ linked (ie the statement 'Otherwise ...' in the function
description).

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer