Re: [PATCH 2/4] tcm_qla2xxx: T10-Dif set harware capability
On 4/1/2014 8:40 PM, Nicholas A. Bellinger wrote: So no way to get it centralized? Yes, still working on how that might look.. I still don't understand the iscsi/iser constraint. Every other fabric aside from iscsi/iser could simply provide a TFO->get_fabric_prot(se_tpg) to query for supported PI. The reason why iscsi/iser is unique is because we can have different network portals (eg: iser protected + traditional iscsi unprotected) on the same se_tpg endpoint. I see. That seems solvable to me... perhaps the easiest thing is to just go ahead and support T10-PI in iscsi-tcp (SW)... Sagi. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] tcm_qla2xxx: T10-Dif set harware capability
On Tue, 2014-04-01 at 11:04 +0300, Sagi Grimberg wrote: > On 4/1/2014 4:11 AM, Nicholas A. Bellinger wrote: > > + > +if (scsi_host_get_prot(lport->qla_vha->host)) { > +tpg->se_tpg.fabric_sup_prot_type = (TARGET_DIF_TYPE0_PROT| > +TARGET_DIF_TYPE1_PROT|TARGET_DIF_TYPE2_PROT| > +TARGET_DIF_TYPE3_PROT); > + > +tpg->se_tpg.fabric_sup_guard_type = TARGET_GUARD_CRC| > +TARGET_GUARD_IP; > +} > > ret = core_tpg_register(&tcm_qla2xxx_fabric_configfs->tf_ops, wwn, > &tpg->se_tpg, tpg, > TRANSPORT_TPG_TYPE_NORMAL); > @@ -1127,6 +1147,8 @@ static ssize_t tcm_qla2xxx_npiv_tpg_store_enable( > qlt_stop_phase1(vha->vha_tgt.qla_tgt); > } > > +core_tpg_set_fabric_t10dif(se_tpg, tpg->tpg_attrib.t10dif_force_on); > + > >>> Any way we can get this logic to be shared also with iscsi, srp, etc... > >>> all fabrics should > >>> be set with t10dif right? so I would imagine it would be better to > >>> centralize it right? > >> QT> Not sure how you want this logic to be shared. This patch is specific > >> to Qlogic driver registering its capabilities. > >> > > I think that Sagi was referring to a target_core_fabric_ops callback to > > query protection information from the fabric.. > > > > As mentioned in the last response, this would work just fine on > > a /sys/kernel/config/target/$FABRIC/$WWPN/$TPGT context basis, if it > > wasn't for iscsi-target / iser-target sharing the same endpoint while > > still allowing different protection modes. > > So no way to get it centralized? Yes, still working on how that might look.. > I still don't understand the iscsi/iser constraint. Every other fabric aside from iscsi/iser could simply provide a TFO->get_fabric_prot(se_tpg) to query for supported PI. The reason why iscsi/iser is unique is because we can have different network portals (eg: iser protected + traditional iscsi unprotected) on the same se_tpg endpoint. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] tcm_qla2xxx: T10-Dif set harware capability
On 4/1/2014 4:11 AM, Nicholas A. Bellinger wrote: + +if (scsi_host_get_prot(lport->qla_vha->host)) { +tpg->se_tpg.fabric_sup_prot_type = (TARGET_DIF_TYPE0_PROT| +TARGET_DIF_TYPE1_PROT|TARGET_DIF_TYPE2_PROT| +TARGET_DIF_TYPE3_PROT); + +tpg->se_tpg.fabric_sup_guard_type = TARGET_GUARD_CRC| +TARGET_GUARD_IP; +} ret = core_tpg_register(&tcm_qla2xxx_fabric_configfs->tf_ops, wwn, &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL); @@ -1127,6 +1147,8 @@ static ssize_t tcm_qla2xxx_npiv_tpg_store_enable( qlt_stop_phase1(vha->vha_tgt.qla_tgt); } +core_tpg_set_fabric_t10dif(se_tpg, tpg->tpg_attrib.t10dif_force_on); + Any way we can get this logic to be shared also with iscsi, srp, etc... all fabrics should be set with t10dif right? so I would imagine it would be better to centralize it right? QT> Not sure how you want this logic to be shared. This patch is specific to Qlogic driver registering its capabilities. I think that Sagi was referring to a target_core_fabric_ops callback to query protection information from the fabric.. As mentioned in the last response, this would work just fine on a /sys/kernel/config/target/$FABRIC/$WWPN/$TPGT context basis, if it wasn't for iscsi-target / iser-target sharing the same endpoint while still allowing different protection modes. So no way to get it centralized? I still don't understand the iscsi/iser constraint. Sagi. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] tcm_qla2xxx: T10-Dif set harware capability
On Mon, 2014-03-31 at 15:38 +, Quinn Tran wrote: > On 3/28/14 5:12 PM, "sagi grimberg" wrote: > > >On 3/29/2014 2:05 AM, Quinn Tran wrote: > >> Set Protection Type(1,2,3) capabilities, Guarg type (CRC/IPchksm) > >> capabilities bits to let TCM core knows of HW/fabric capabilities. > >> > >> Signed-off-by: Nicholas Bellinger > >> Signed-off-by: Giridhar Malavali > >> --- > >> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 23 +++ > >> drivers/scsi/qla2xxx/tcm_qla2xxx.h | 1 + > >> 2 files changed, 24 insertions(+) > >> > >> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c > >>b/drivers/scsi/qla2xxx/tcm_qla2xxx.c > >> index b23a0ff..4d93081 100644 > >> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c > >> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c > >> + > >> +if (scsi_host_get_prot(lport->qla_vha->host)) { > >> +tpg->se_tpg.fabric_sup_prot_type = (TARGET_DIF_TYPE0_PROT| > >> +TARGET_DIF_TYPE1_PROT|TARGET_DIF_TYPE2_PROT| > >> +TARGET_DIF_TYPE3_PROT); > >> + > >> +tpg->se_tpg.fabric_sup_guard_type = TARGET_GUARD_CRC| > >> +TARGET_GUARD_IP; > >> +} > >> > >> ret = core_tpg_register(&tcm_qla2xxx_fabric_configfs->tf_ops, wwn, > >> &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL); > >> @@ -1127,6 +1147,8 @@ static ssize_t tcm_qla2xxx_npiv_tpg_store_enable( > >> qlt_stop_phase1(vha->vha_tgt.qla_tgt); > >> } > >> > >> +core_tpg_set_fabric_t10dif(se_tpg, tpg->tpg_attrib.t10dif_force_on); > >> + > > > >Any way we can get this logic to be shared also with iscsi, srp, etc... > >all fabrics should > >be set with t10dif right? so I would imagine it would be better to > >centralize it right? > > QT> Not sure how you want this logic to be shared. This patch is specific > to Qlogic driver registering its capabilities. > I think that Sagi was referring to a target_core_fabric_ops callback to query protection information from the fabric.. As mentioned in the last response, this would work just fine on a /sys/kernel/config/target/$FABRIC/$WWPN/$TPGT context basis, if it wasn't for iscsi-target / iser-target sharing the same endpoint while still allowing different protection modes. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] tcm_qla2xxx: T10-Dif set harware capability
Regards, Quinn Tran On 3/28/14 5:12 PM, "sagi grimberg" wrote: >On 3/29/2014 2:05 AM, Quinn Tran wrote: >> Set Protection Type(1,2,3) capabilities, Guarg type (CRC/IPchksm) >> capabilities bits to let TCM core knows of HW/fabric capabilities. >> >> Signed-off-by: Nicholas Bellinger >> Signed-off-by: Giridhar Malavali >> --- >> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 23 +++ >> drivers/scsi/qla2xxx/tcm_qla2xxx.h | 1 + >> 2 files changed, 24 insertions(+) >> >> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c >>b/drivers/scsi/qla2xxx/tcm_qla2xxx.c >> index b23a0ff..4d93081 100644 >> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c >> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c >> @@ -910,12 +910,20 @@ DEF_QLA_TPG_ATTR_BOOL(demo_mode_login_only); >> DEF_QLA_TPG_ATTRIB(demo_mode_login_only); >> QLA_TPG_ATTR(demo_mode_login_only, S_IRUGO | S_IWUSR); >> >> +/* >> + * Define tcm_qla2xxx_tpg_attrib_s_t10dif_force_on >> + */ >> +DEF_QLA_TPG_ATTR_BOOL(t10dif_force_on); >> +DEF_QLA_TPG_ATTRIB(t10dif_force_on); >> +QLA_TPG_ATTR(t10dif_force_on, S_IRUGO | S_IWUSR); >> + >> static struct configfs_attribute *tcm_qla2xxx_tpg_attrib_attrs[] = { >> &tcm_qla2xxx_tpg_attrib_generate_node_acls.attr, >> &tcm_qla2xxx_tpg_attrib_cache_dynamic_acls.attr, >> &tcm_qla2xxx_tpg_attrib_demo_mode_write_protect.attr, >> &tcm_qla2xxx_tpg_attrib_prod_mode_write_protect.attr, >> &tcm_qla2xxx_tpg_attrib_demo_mode_login_only.attr, >> +&tcm_qla2xxx_tpg_attrib_t10dif_force_on.attr, >> NULL, >> }; >> >> @@ -1049,6 +1057,18 @@ static struct se_portal_group >>*tcm_qla2xxx_make_tpg( >> tpg->tpg_attrib.demo_mode_write_protect = 1; >> tpg->tpg_attrib.cache_dynamic_acls = 1; >> tpg->tpg_attrib.demo_mode_login_only = 1; >> +tpg->tpg_attrib.t10dif_force_on = 0; >> +tpg->se_tpg.fabric_sup_prot_type = 0; >> +tpg->se_tpg.fabric_sup_guard_type = 0; > >You can lose guard_type - this is more relevant to the initiator side. QT> OK > >> + >> +if (scsi_host_get_prot(lport->qla_vha->host)) { >> +tpg->se_tpg.fabric_sup_prot_type = (TARGET_DIF_TYPE0_PROT| >> +TARGET_DIF_TYPE1_PROT|TARGET_DIF_TYPE2_PROT| >> +TARGET_DIF_TYPE3_PROT); >> + >> +tpg->se_tpg.fabric_sup_guard_type = TARGET_GUARD_CRC| >> +TARGET_GUARD_IP; >> +} >> >> ret = core_tpg_register(&tcm_qla2xxx_fabric_configfs->tf_ops, wwn, >> &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL); >> @@ -1127,6 +1147,8 @@ static ssize_t tcm_qla2xxx_npiv_tpg_store_enable( >> qlt_stop_phase1(vha->vha_tgt.qla_tgt); >> } >> >> +core_tpg_set_fabric_t10dif(se_tpg, tpg->tpg_attrib.t10dif_force_on); >> + > >Any way we can get this logic to be shared also with iscsi, srp, etc... >all fabrics should >be set with t10dif right? so I would imagine it would be better to >centralize it right? QT> Not sure how you want this logic to be shared. This patch is specific to Qlogic driver registering its capabilities. > >> return count; >> } >> >> @@ -1169,6 +1191,7 @@ static struct se_portal_group >>*tcm_qla2xxx_npiv_make_tpg( >> tpg->tpg_attrib.demo_mode_write_protect = 1; >> tpg->tpg_attrib.cache_dynamic_acls = 1; >> tpg->tpg_attrib.demo_mode_login_only = 1; >> +tpg->tpg_attrib.t10dif_force_on = 0; >> >> ret = core_tpg_register(&tcm_qla2xxx_npiv_fabric_configfs->tf_ops, >>wwn, >> &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL); >> diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.h >>b/drivers/scsi/qla2xxx/tcm_qla2xxx.h >> index 33aaac8..62fdce3 100644 >> --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.h >> +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.h >> @@ -28,6 +28,7 @@ struct tcm_qla2xxx_tpg_attrib { >> int demo_mode_write_protect; >> int prod_mode_write_protect; >> int demo_mode_login_only; >> +int t10dif_force_on; >> }; >> >> struct tcm_qla2xxx_tpg { This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message. <>
Re: [PATCH 2/4] tcm_qla2xxx: T10-Dif set harware capability
On 3/29/2014 2:05 AM, Quinn Tran wrote: Set Protection Type(1,2,3) capabilities, Guarg type (CRC/IPchksm) capabilities bits to let TCM core knows of HW/fabric capabilities. Signed-off-by: Nicholas Bellinger Signed-off-by: Giridhar Malavali --- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 23 +++ drivers/scsi/qla2xxx/tcm_qla2xxx.h | 1 + 2 files changed, 24 insertions(+) diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index b23a0ff..4d93081 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -910,12 +910,20 @@ DEF_QLA_TPG_ATTR_BOOL(demo_mode_login_only); DEF_QLA_TPG_ATTRIB(demo_mode_login_only); QLA_TPG_ATTR(demo_mode_login_only, S_IRUGO | S_IWUSR); +/* + * Define tcm_qla2xxx_tpg_attrib_s_t10dif_force_on + */ +DEF_QLA_TPG_ATTR_BOOL(t10dif_force_on); +DEF_QLA_TPG_ATTRIB(t10dif_force_on); +QLA_TPG_ATTR(t10dif_force_on, S_IRUGO | S_IWUSR); + static struct configfs_attribute *tcm_qla2xxx_tpg_attrib_attrs[] = { &tcm_qla2xxx_tpg_attrib_generate_node_acls.attr, &tcm_qla2xxx_tpg_attrib_cache_dynamic_acls.attr, &tcm_qla2xxx_tpg_attrib_demo_mode_write_protect.attr, &tcm_qla2xxx_tpg_attrib_prod_mode_write_protect.attr, &tcm_qla2xxx_tpg_attrib_demo_mode_login_only.attr, + &tcm_qla2xxx_tpg_attrib_t10dif_force_on.attr, NULL, }; @@ -1049,6 +1057,18 @@ static struct se_portal_group *tcm_qla2xxx_make_tpg( tpg->tpg_attrib.demo_mode_write_protect = 1; tpg->tpg_attrib.cache_dynamic_acls = 1; tpg->tpg_attrib.demo_mode_login_only = 1; + tpg->tpg_attrib.t10dif_force_on = 0; + tpg->se_tpg.fabric_sup_prot_type = 0; + tpg->se_tpg.fabric_sup_guard_type = 0; You can lose guard_type - this is more relevant to the initiator side. + + if (scsi_host_get_prot(lport->qla_vha->host)) { + tpg->se_tpg.fabric_sup_prot_type = (TARGET_DIF_TYPE0_PROT| + TARGET_DIF_TYPE1_PROT|TARGET_DIF_TYPE2_PROT| + TARGET_DIF_TYPE3_PROT); + + tpg->se_tpg.fabric_sup_guard_type = TARGET_GUARD_CRC| + TARGET_GUARD_IP; + } ret = core_tpg_register(&tcm_qla2xxx_fabric_configfs->tf_ops, wwn, &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL); @@ -1127,6 +1147,8 @@ static ssize_t tcm_qla2xxx_npiv_tpg_store_enable( qlt_stop_phase1(vha->vha_tgt.qla_tgt); } + core_tpg_set_fabric_t10dif(se_tpg, tpg->tpg_attrib.t10dif_force_on); + Any way we can get this logic to be shared also with iscsi, srp, etc... all fabrics should be set with t10dif right? so I would imagine it would be better to centralize it right? return count; } @@ -1169,6 +1191,7 @@ static struct se_portal_group *tcm_qla2xxx_npiv_make_tpg( tpg->tpg_attrib.demo_mode_write_protect = 1; tpg->tpg_attrib.cache_dynamic_acls = 1; tpg->tpg_attrib.demo_mode_login_only = 1; + tpg->tpg_attrib.t10dif_force_on = 0; ret = core_tpg_register(&tcm_qla2xxx_npiv_fabric_configfs->tf_ops, wwn, &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL); diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.h b/drivers/scsi/qla2xxx/tcm_qla2xxx.h index 33aaac8..62fdce3 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.h +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.h @@ -28,6 +28,7 @@ struct tcm_qla2xxx_tpg_attrib { int demo_mode_write_protect; int prod_mode_write_protect; int demo_mode_login_only; + int t10dif_force_on; }; struct tcm_qla2xxx_tpg { -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] tcm_qla2xxx: T10-Dif set harware capability
Set Protection Type(1,2,3) capabilities, Guarg type (CRC/IPchksm) capabilities bits to let TCM core knows of HW/fabric capabilities. Signed-off-by: Nicholas Bellinger Signed-off-by: Giridhar Malavali --- drivers/scsi/qla2xxx/tcm_qla2xxx.c | 23 +++ drivers/scsi/qla2xxx/tcm_qla2xxx.h | 1 + 2 files changed, 24 insertions(+) diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index b23a0ff..4d93081 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -910,12 +910,20 @@ DEF_QLA_TPG_ATTR_BOOL(demo_mode_login_only); DEF_QLA_TPG_ATTRIB(demo_mode_login_only); QLA_TPG_ATTR(demo_mode_login_only, S_IRUGO | S_IWUSR); +/* + * Define tcm_qla2xxx_tpg_attrib_s_t10dif_force_on + */ +DEF_QLA_TPG_ATTR_BOOL(t10dif_force_on); +DEF_QLA_TPG_ATTRIB(t10dif_force_on); +QLA_TPG_ATTR(t10dif_force_on, S_IRUGO | S_IWUSR); + static struct configfs_attribute *tcm_qla2xxx_tpg_attrib_attrs[] = { &tcm_qla2xxx_tpg_attrib_generate_node_acls.attr, &tcm_qla2xxx_tpg_attrib_cache_dynamic_acls.attr, &tcm_qla2xxx_tpg_attrib_demo_mode_write_protect.attr, &tcm_qla2xxx_tpg_attrib_prod_mode_write_protect.attr, &tcm_qla2xxx_tpg_attrib_demo_mode_login_only.attr, + &tcm_qla2xxx_tpg_attrib_t10dif_force_on.attr, NULL, }; @@ -1049,6 +1057,18 @@ static struct se_portal_group *tcm_qla2xxx_make_tpg( tpg->tpg_attrib.demo_mode_write_protect = 1; tpg->tpg_attrib.cache_dynamic_acls = 1; tpg->tpg_attrib.demo_mode_login_only = 1; + tpg->tpg_attrib.t10dif_force_on = 0; + tpg->se_tpg.fabric_sup_prot_type = 0; + tpg->se_tpg.fabric_sup_guard_type = 0; + + if (scsi_host_get_prot(lport->qla_vha->host)) { + tpg->se_tpg.fabric_sup_prot_type = (TARGET_DIF_TYPE0_PROT| + TARGET_DIF_TYPE1_PROT|TARGET_DIF_TYPE2_PROT| + TARGET_DIF_TYPE3_PROT); + + tpg->se_tpg.fabric_sup_guard_type = TARGET_GUARD_CRC| + TARGET_GUARD_IP; + } ret = core_tpg_register(&tcm_qla2xxx_fabric_configfs->tf_ops, wwn, &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL); @@ -1127,6 +1147,8 @@ static ssize_t tcm_qla2xxx_npiv_tpg_store_enable( qlt_stop_phase1(vha->vha_tgt.qla_tgt); } + core_tpg_set_fabric_t10dif(se_tpg, tpg->tpg_attrib.t10dif_force_on); + return count; } @@ -1169,6 +1191,7 @@ static struct se_portal_group *tcm_qla2xxx_npiv_make_tpg( tpg->tpg_attrib.demo_mode_write_protect = 1; tpg->tpg_attrib.cache_dynamic_acls = 1; tpg->tpg_attrib.demo_mode_login_only = 1; + tpg->tpg_attrib.t10dif_force_on = 0; ret = core_tpg_register(&tcm_qla2xxx_npiv_fabric_configfs->tf_ops, wwn, &tpg->se_tpg, tpg, TRANSPORT_TPG_TYPE_NORMAL); diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.h b/drivers/scsi/qla2xxx/tcm_qla2xxx.h index 33aaac8..62fdce3 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.h +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.h @@ -28,6 +28,7 @@ struct tcm_qla2xxx_tpg_attrib { int demo_mode_write_protect; int prod_mode_write_protect; int demo_mode_login_only; + int t10dif_force_on; }; struct tcm_qla2xxx_tpg { -- 1.8.4.GIT -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html