From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752861AbdK3PIw (ORCPT ); Thu, 30 Nov 2017 10:08:52 -0500 Received: from mx2.suse.de ([195.135.220.15]:49987 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752222AbdK3PIv (ORCPT ); Thu, 30 Nov 2017 10:08:51 -0500 Subject: Re: [PATCH] nvme-fc: don't require user to enter host_traddr To: Johannes Thumshirn , Christoph Hellwig Cc: Linux Kernel Mailinglist , Sagi Grimberg , Keith Busch , Linux NVMe Mailinglist , James Smart References: <20171130145615.7506-1-jthumshirn@suse.de> From: Hannes Reinecke Message-ID: Date: Thu, 30 Nov 2017 16:08:49 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <20171130145615.7506-1-jthumshirn@suse.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/30/2017 03:56 PM, Johannes Thumshirn wrote: > One major usability difference between NVMf RDMA and FC is resolving > the default host transport address in RDMA. This is perfectly doable > in FC as well, as we already have all possible lport <-> rport > combinations pre-populated so we can pick the first lport that has a > connection to our desired rport per default or optionally use the user > supplied lport if we have one. > > Signed-off-by: Johannes Thumshirn > Cc: James Smart > --- > drivers/nvme/host/fc.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c > index 7ab0be55c7d0..46ab900bbe26 100644 > --- a/drivers/nvme/host/fc.c > +++ b/drivers/nvme/host/fc.c > @@ -3328,15 +3328,19 @@ nvme_fc_create_ctrl(struct device *dev, struct nvmf_ctrl_options *opts) > if (ret || !raddr.nn || !raddr.pn) > return ERR_PTR(-EINVAL); > > - ret = nvme_fc_parse_traddr(&laddr, opts->host_traddr, NVMF_TRADDR_SIZE); > - if (ret || !laddr.nn || !laddr.pn) > - return ERR_PTR(-EINVAL); > + if (opts->mask & NVMF_OPT_HOST_TRADDR) { > + ret = nvme_fc_parse_traddr(&laddr, opts->host_traddr, > + NVMF_TRADDR_SIZE); > + if (ret || !laddr.nn || !laddr.pn) > + return ERR_PTR(-EINVAL); > + } > > /* find the host and remote ports to connect together */ > spin_lock_irqsave(&nvme_fc_lock, flags); > list_for_each_entry(lport, &nvme_fc_lport_list, port_list) { > - if (lport->localport.node_name != laddr.nn || > - lport->localport.port_name != laddr.pn) > + if ((laddr.nn || laddr.pn) && > + (lport->localport.node_name != laddr.nn || > + lport->localport.port_name != laddr.pn)) > continue; > > list_for_each_entry(rport, &lport->endp_list, endp_list) { We need both, 'laddr.nn' and 'laddr.pn'. So this statement is wrong. You probably need something like if (!laddr.nn || !laddr.pn || ....) Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)