Hi Bart,
> On Nov 12, 2018, at 2:06 PM, Bart Van Assche <bvanassche@xxxxxxx> wrote:
>
> External Email
>
> On Mon, 2018-11-12 at 13:40 -0800, Himanshu Madhani wrote:
>> diff --git a/drivers/scsi/qla2xxx/qla_attr.c
>> b/drivers/scsi/qla2xxx/qla_attr.c
>> index 678aff5ca947..323a4aa35f16 100644
>> --- a/drivers/scsi/qla2xxx/qla_attr.c
>> +++ b/drivers/scsi/qla2xxx/qla_attr.c
>> @@ -1665,6 +1665,125 @@ qla2x00_max_speed_sup_show(struct device *dev,
>> struct device_attribute *attr,
>> ha->max_speed_sup ? "32Gps" : "16Gps");
>> }
>>
>> +static ssize_t
>> +qla27xx_nvme_connect_str_show(struct device *dev, struct device_attribute
>> *attr,
>> + char *buf)
>> +{
>> + scsi_qla_host_t *vha = shost_priv(class_to_shost(dev));
>> + struct nvme_fc_remote_port *rport;
>> + struct nvme_fc_local_port *lport;
>> + struct qla_hw_data *ha = vha->hw;
>> + struct qla_nvme_rport *qla_rport, *trport;
>> + fc_port_t *fcport;
>> + char temp[150] = {0};
>> + char *rportstate = "";
>> +
>> + if (!IS_QLA27XX(ha) && !IS_QLA28XX(ha))
>> + return scnprintf(buf, PAGE_SIZE, "\n");
>> +
>> + if (!vha->flags.nvme_enabled)
>> + return scnprintf(buf, PAGE_SIZE, "%s\n",
>> + "FC-NVMe is not enabled");
>> +
>> + list_for_each_entry(fcport, &vha->vp_fcports, list) {
>> + if (!fcport) {
>> + scnprintf(buf, PAGE_SIZE, "No FC host\n");
>> + return strlen(buf);
>> + }
>> +
>> + if (!vha->nvme_local_port) {
>> + scnprintf(buf, PAGE_SIZE,
>> + "FC-NVMe Initiator on 0x%16llx not
>> registered.\n",
>> + wwn_to_u64(fcport->port_name));
>> + return strlen(buf);
>> + }
>> +
>> + list_for_each_entry_safe(qla_rport, trport,
>> + &vha->nvme_rport_list, list) {
>> + if (qla_rport->fcport == fcport) {
>> + rport = fcport->nvme_remote_port;
>> +
>> + lport = vha->nvme_local_port;
>> +
>> + scnprintf(temp, sizeof(temp),
>> + "FC-NVMe LPORT: host%ld nn-
>> 0x%16llx:pn-0x%16llx port_id %06x %s\n",
>> + vha->host_no, lport->node_name,
>> + lport->port_name, lport->port_id,
>> "ONLINE");
>> +
>> + if (strlcat(buf, temp, PAGE_SIZE) >=
>> PAGE_SIZE)
>> + goto done;
>> +
>> + scnprintf(temp, sizeof(temp),
>> + "FC-NVMe RPORT: host%ld nn-0x%llx:pn-
>> 0x%llx port_id %06x ",
>> + vha->host_no, rport->node_name,
>> + rport->port_name, rport->port_id);
>> +
>> + /* Find out Rport State */
>> + if (rport->port_state &
>> FC_OBJSTATE_ONLINE)
>> + rportstate = "ONLINE";
>> +
>> + if (rport->port_state &
>> FC_OBJSTATE_UNKNOWN)
>> + rportstate = "UNKNOWN";
>> +
>> + if (rport->port_state &
>> ~(FC_OBJSTATE_ONLINE |
>> + FC_OBJSTATE_UNKNOWN))
>> + rportstate = "UNSUPPORTED";
>> +
>> + if (strlcat(buf, temp, PAGE_SIZE) >=
>> + PAGE_SIZE)
>> + goto done;
>> +
>> + if (rport->port_role &
>> + (FC_PORT_ROLE_NVME_INITIATOR |
>> + FC_PORT_ROLE_NVME_TARGET |
>> + FC_PORT_ROLE_NVME_DISCOVERY)) {
>> + if (rport->port_role &
>> + FC_PORT_ROLE_NVME_INITIATOR)
>> + if (strlcat(buf,
>> "INITIATOR ",
>> + PAGE_SIZE) >=
>> PAGE_SIZE)
>> + goto done;
>> +
>> + if (rport->port_role &
>> + FC_PORT_ROLE_NVME_TARGET)
>> + if (strlcat(buf, "TARGET
>> ",
>> + PAGE_SIZE) >=
>> PAGE_SIZE)
>> + goto done;
>> +
>> + if (rport->port_role &
>> + FC_PORT_ROLE_NVME_DISCOVERY)
>> + if (strlcat(buf,
>> "DISCOVERY ",
>> + PAGE_SIZE) >=
>> PAGE_SIZE)
>> + goto done;
>> + } else {
>> + if (strlcat(buf, "UNKNOWN_ROLE ",
>> + PAGE_SIZE) >= PAGE_SIZE)
>> + goto done;
>> + }
>> + scnprintf(temp, sizeof(temp), "%s\n",
>> rportstate);
>> +
>> + if (strlcat(buf, temp, PAGE_SIZE) >=
>> PAGE_SIZE)
>> + goto done;
>> +
>> + scnprintf(temp, sizeof(temp),
>> + "NVMECLI: host-traddr=nn-0x%16llx:pn-
>> 0x%16llx traddr=nn-0x%16llx:pn-0x%16llx\n",
>> + lport->node_name, lport->port_name,
>> + rport->node_name, rport->port_name);
>> +
>> + if (strlcat(buf, temp, PAGE_SIZE) >=
>> PAGE_SIZE)
>> + goto done;
>> + }
>> + }
>> + }
>> +
>> + return strlen(buf);
>> +
>> +done:
>> + ql_log(ql_log_warn, vha, 0xffff,
>> + "NVME connect string buffer size 0x%lx exceeds 0x%lx\n",
>> + sizeof(*buf), PAGE_SIZE);
>> + return strlen(buf);
>> +}
>
> Hi Himanshu,
>
> Are you familiar with the "one value per file" rule for sysfs? A quote from
> Documentation/filesystems/sysfs.txt: "Attributes should be ASCII text files,
> preferably with only one value per file."
>
> Thanks,
>
> Bart.
>
Yes. Thanks for pointing out documentation related to SysFS.
The main reason this was done to allow NVMe CLI to be able to take string with
host-traddr/traddr in one shot instead of parsing multiple SysFS hooks.
I see other drivers also use similar information populated for NVMe Connection at boot time.
Thanks,
- Himanshu
