| Message ID | 1452457724-10629-2-git-send-email-nab@daterainc.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 01/10/2016 12:28 PM, Nicholas A. Bellinger wrote: > This patch does a simple conversion of tcm_fc code to use > proper modern core_tpg_get_initiator_node_acl() lookup using > se_node_acl->acl_kref, and drops the legacy list walk from > ft_acl_get(). > > Note the original lookup also took node_name into account, > but since ft_init_nodeacl() only ever sets port_name for > se_node_acl->acl_group within configfs, this is purely > a mechanical change. Hi Nic, This patch modifies ft_prli_locked() such that the ACL list lookup only happens if FC_SPP_EST_IMG_PAIR has been set by the initiator. That is a behavior change and this has not been mentioned in the patch description. I think the patch description should mention that behavior change and also that it should explain why that behavior change is acceptable - if it is acceptable. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 10, 2016 at 08:28:41PM +0000, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger <nab@linux-iscsi.org> > > This patch does a simple conversion of tcm_fc code to use > proper modern core_tpg_get_initiator_node_acl() lookup using > se_node_acl->acl_kref, and drops the legacy list walk from > ft_acl_get(). > > Note the original lookup also took node_name into account, > but since ft_init_nodeacl() only ever sets port_name for > se_node_acl->acl_group within configfs, this is purely > a mechanical change. Please remove ft_acl_get and fold it's new implementation into the caller. The later patches actually remove the usage of it but keep it around, but it would be much better to kill it off here. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2016-01-12 at 16:02 +0100, Christoph Hellwig wrote: > On Sun, Jan 10, 2016 at 08:28:41PM +0000, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger <nab@linux-iscsi.org> > > > > This patch does a simple conversion of tcm_fc code to use > > proper modern core_tpg_get_initiator_node_acl() lookup using > > se_node_acl->acl_kref, and drops the legacy list walk from > > ft_acl_get(). > > > > Note the original lookup also took node_name into account, > > but since ft_init_nodeacl() only ever sets port_name for > > se_node_acl->acl_group within configfs, this is purely > > a mechanical change. > > Please remove ft_acl_get and fold it's new implementation into the > caller. The later patches actually remove the usage of it but keep it > around, but it would be much better to kill it off here. Done. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/target/tcm_fc/tfc_conf.c b/drivers/target/tcm_fc/tfc_conf.c index 9cdb2ac..9389ba3 100644 --- a/drivers/target/tcm_fc/tfc_conf.c +++ b/drivers/target/tcm_fc/tfc_conf.c @@ -222,27 +222,17 @@ static int ft_init_nodeacl(struct se_node_acl *nacl, const char *name) struct ft_node_acl *ft_acl_get(struct ft_tpg *tpg, struct fc_rport_priv *rdata) { - struct ft_node_acl *found = NULL; - struct ft_node_acl *acl; struct se_portal_group *se_tpg = &tpg->se_tpg; struct se_node_acl *se_acl; + unsigned char initiatorname[TRANSPORT_IQN_LEN]; - mutex_lock(&se_tpg->acl_node_mutex); - list_for_each_entry(se_acl, &se_tpg->acl_node_list, acl_list) { - acl = container_of(se_acl, struct ft_node_acl, se_node_acl); - pr_debug("acl %p port_name %llx\n", - acl, (unsigned long long)acl->node_auth.port_name); - if (acl->node_auth.port_name == rdata->ids.port_name || - acl->node_auth.node_name == rdata->ids.node_name) { - pr_debug("acl %p port_name %llx matched\n", acl, - (unsigned long long)rdata->ids.port_name); - found = acl; - /* XXX need to hold onto ACL */ - break; - } - } - mutex_unlock(&se_tpg->acl_node_mutex); - return found; + ft_format_wwn(&initiatorname[0], TRANSPORT_IQN_LEN, rdata->ids.port_name); + + se_acl = core_tpg_get_initiator_node_acl(se_tpg, &initiatorname[0]); + if (!se_acl) + return NULL; + + return container_of(se_acl, struct ft_node_acl, se_node_acl); } /* diff --git a/drivers/target/tcm_fc/tfc_sess.c b/drivers/target/tcm_fc/tfc_sess.c index 45947e2..b3db638 100644 --- a/drivers/target/tcm_fc/tfc_sess.c +++ b/drivers/target/tcm_fc/tfc_sess.c @@ -191,9 +191,10 @@ out: * Caller holds ft_lport_lock. */ static struct ft_sess *ft_sess_create(struct ft_tport *tport, u32 port_id, - struct ft_node_acl *acl) + struct fc_rport_priv *rdata) { struct ft_sess *sess; + struct ft_node_acl *acl; struct hlist_head *head; head = &tport->hash[ft_sess_hash(port_id)]; @@ -212,6 +213,14 @@ static struct ft_sess *ft_sess_create(struct ft_tport *tport, u32 port_id, kfree(sess); return NULL; } + + acl = ft_acl_get(tport->tpg, rdata); + if (!acl) { + transport_free_session(sess->se_sess); + kfree(sess); + return NULL; + } + sess->se_sess->se_node_acl = &acl->se_node_acl; sess->tport = tport; sess->port_id = port_id; @@ -349,17 +358,12 @@ static int ft_prli_locked(struct fc_rport_priv *rdata, u32 spp_len, { struct ft_tport *tport; struct ft_sess *sess; - struct ft_node_acl *acl; u32 fcp_parm; tport = ft_tport_get(rdata->local_port); if (!tport) goto not_target; /* not a target for this local port */ - acl = ft_acl_get(tport->tpg, rdata); - if (!acl) - goto not_target; /* no target for this remote */ - if (!rspp) goto fill; @@ -381,7 +385,7 @@ static int ft_prli_locked(struct fc_rport_priv *rdata, u32 spp_len, spp->spp_flags |= FC_SPP_EST_IMG_PAIR; if (!(fcp_parm & FCP_SPPF_INIT_FCN)) return FC_SPP_RESP_CONF; - sess = ft_sess_create(tport, rdata->ids.port_id, acl); + sess = ft_sess_create(tport, rdata->ids.port_id, rdata); if (!sess) return FC_SPP_RESP_RES; if (!sess->params)