* [PATCH v10 01/11] fc_els: use 'union fc_tlv_desc'
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-29 17:44 ` Justin Tee
2025-09-26 0:01 ` [PATCH v10 02/11] nvme: add NVME_CTRL_MARGINAL flag John Meneghini
` (9 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
From: Hannes Reinecke <hare@kernel.org>
Introduce 'union fc_tlv_desc' to have a common structure for all FC
ELS TLV structures and avoid type casts.
[bgurney: The cast inside the union fc_tlv_next_desc() has "u8",
which causes a failure to build. Use "__u8" instead.]
Signed-off-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Justin Tee <justin.tee@broadcom.com>
Tested-by: Bryan Gurney <bgurney@redhat.com>
Reviewed-by: John Meneghini <jmeneghi@redhat.com>
Tested-by: Muneendra Kumar <muneendra.kumar@broadcom.com>
---
drivers/scsi/lpfc/lpfc_els.c | 75 +++++++-------
drivers/scsi/scsi_transport_fc.c | 27 +++--
include/uapi/scsi/fc/fc_els.h | 165 +++++++++++++++++--------------
3 files changed, 135 insertions(+), 132 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index fca81e0c7c2e..1511cabac636 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -3937,7 +3937,7 @@ lpfc_cmpl_els_edc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
{
IOCB_t *irsp_iocb;
struct fc_els_edc_resp *edc_rsp;
- struct fc_tlv_desc *tlv;
+ union fc_tlv_desc *tlv;
struct fc_diag_cg_sig_desc *pcgd;
struct fc_diag_lnkflt_desc *plnkflt;
struct lpfc_dmabuf *pcmd, *prsp;
@@ -4028,7 +4028,7 @@ lpfc_cmpl_els_edc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
goto out;
}
- dtag = be32_to_cpu(tlv->desc_tag);
+ dtag = be32_to_cpu(tlv->hdr.desc_tag);
switch (dtag) {
case ELS_DTAG_LNK_FAULT_CAP:
if (bytes_remain < FC_TLV_DESC_SZ_FROM_LENGTH(tlv) ||
@@ -4043,7 +4043,7 @@ lpfc_cmpl_els_edc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
sizeof(struct fc_diag_lnkflt_desc));
goto out;
}
- plnkflt = (struct fc_diag_lnkflt_desc *)tlv;
+ plnkflt = &tlv->lnkflt;
lpfc_printf_log(phba, KERN_INFO,
LOG_ELS | LOG_LDS_EVENT,
"4617 Link Fault Desc Data: 0x%08x 0x%08x "
@@ -4070,7 +4070,7 @@ lpfc_cmpl_els_edc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
goto out;
}
- pcgd = (struct fc_diag_cg_sig_desc *)tlv;
+ pcgd = &tlv->cg_sig;
lpfc_printf_log(
phba, KERN_INFO, LOG_ELS | LOG_CGN_MGMT,
"4616 CGN Desc Data: 0x%08x 0x%08x "
@@ -4125,10 +4125,8 @@ lpfc_cmpl_els_edc(struct lpfc_hba *phba, struct lpfc_iocbq *cmdiocb,
}
static void
-lpfc_format_edc_lft_desc(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
+lpfc_format_edc_lft_desc(struct lpfc_hba *phba, struct fc_diag_lnkflt_desc *lft)
{
- struct fc_diag_lnkflt_desc *lft = (struct fc_diag_lnkflt_desc *)tlv;
-
lft->desc_tag = cpu_to_be32(ELS_DTAG_LNK_FAULT_CAP);
lft->desc_len = cpu_to_be32(
FC_TLV_DESC_LENGTH_FROM_SZ(struct fc_diag_lnkflt_desc));
@@ -4141,10 +4139,8 @@ lpfc_format_edc_lft_desc(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
}
static void
-lpfc_format_edc_cgn_desc(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
+lpfc_format_edc_cgn_desc(struct lpfc_hba *phba, struct fc_diag_cg_sig_desc *cgd)
{
- struct fc_diag_cg_sig_desc *cgd = (struct fc_diag_cg_sig_desc *)tlv;
-
/* We are assuming cgd was zero'ed before calling this routine */
/* Configure the congestion detection capability */
@@ -4233,7 +4229,7 @@ lpfc_issue_els_edc(struct lpfc_vport *vport, uint8_t retry)
struct lpfc_hba *phba = vport->phba;
struct lpfc_iocbq *elsiocb;
struct fc_els_edc *edc_req;
- struct fc_tlv_desc *tlv;
+ union fc_tlv_desc *tlv;
u16 cmdsize;
struct lpfc_nodelist *ndlp;
u8 *pcmd = NULL;
@@ -4272,13 +4268,13 @@ lpfc_issue_els_edc(struct lpfc_vport *vport, uint8_t retry)
tlv = edc_req->desc;
if (cgn_desc_size) {
- lpfc_format_edc_cgn_desc(phba, tlv);
+ lpfc_format_edc_cgn_desc(phba, &tlv->cg_sig);
phba->cgn_sig_freq = lpfc_fabric_cgn_frequency;
tlv = fc_tlv_next_desc(tlv);
}
if (lft_desc_size)
- lpfc_format_edc_lft_desc(phba, tlv);
+ lpfc_format_edc_lft_desc(phba, &tlv->lnkflt);
lpfc_printf_vlog(vport, KERN_INFO, LOG_ELS | LOG_CGN_MGMT,
"4623 Xmit EDC to remote "
@@ -5824,7 +5820,7 @@ lpfc_issue_els_edc_rsp(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
{
struct lpfc_hba *phba = vport->phba;
struct fc_els_edc_resp *edc_rsp;
- struct fc_tlv_desc *tlv;
+ union fc_tlv_desc *tlv;
struct lpfc_iocbq *elsiocb;
IOCB_t *icmd, *cmd;
union lpfc_wqe128 *wqe;
@@ -5868,10 +5864,10 @@ lpfc_issue_els_edc_rsp(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
FC_TLV_DESC_LENGTH_FROM_SZ(struct fc_els_lsri_desc));
edc_rsp->lsri.rqst_w0.cmd = ELS_EDC;
tlv = edc_rsp->desc;
- lpfc_format_edc_cgn_desc(phba, tlv);
+ lpfc_format_edc_cgn_desc(phba, &tlv->cg_sig);
tlv = fc_tlv_next_desc(tlv);
if (lft_desc_size)
- lpfc_format_edc_lft_desc(phba, tlv);
+ lpfc_format_edc_lft_desc(phba, &tlv->lnkflt);
lpfc_debugfs_disc_trc(vport, LPFC_DISC_TRC_ELS_RSP,
"Issue EDC ACC: did:x%x flg:x%lx refcnt %d",
@@ -9263,7 +9259,7 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
{
struct lpfc_hba *phba = vport->phba;
struct fc_els_edc *edc_req;
- struct fc_tlv_desc *tlv;
+ union fc_tlv_desc *tlv;
uint8_t *payload;
uint32_t *ptr, dtag;
const char *dtag_nm;
@@ -9306,7 +9302,7 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
goto out;
}
- dtag = be32_to_cpu(tlv->desc_tag);
+ dtag = be32_to_cpu(tlv->hdr.desc_tag);
switch (dtag) {
case ELS_DTAG_LNK_FAULT_CAP:
if (bytes_remain < FC_TLV_DESC_SZ_FROM_LENGTH(tlv) ||
@@ -9321,7 +9317,7 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
sizeof(struct fc_diag_lnkflt_desc));
goto out;
}
- plnkflt = (struct fc_diag_lnkflt_desc *)tlv;
+ plnkflt = &tlv->lnkflt;
lpfc_printf_log(phba, KERN_INFO,
LOG_ELS | LOG_LDS_EVENT,
"4626 Link Fault Desc Data: x%08x len x%x "
@@ -9358,7 +9354,7 @@ lpfc_els_rcv_edc(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb,
phba->cgn_sig_freq = lpfc_fabric_cgn_frequency;
lpfc_least_capable_settings(
- phba, (struct fc_diag_cg_sig_desc *)tlv);
+ phba, &tlv->cg_sig);
break;
default:
dtag_nm = lpfc_get_tlv_dtag_nm(dtag);
@@ -9949,14 +9945,13 @@ lpfc_display_fpin_wwpn(struct lpfc_hba *phba, __be64 *wwnlist, u32 cnt)
/**
* lpfc_els_rcv_fpin_li - Process an FPIN Link Integrity Event.
* @phba: Pointer to phba object.
- * @tlv: Pointer to the Link Integrity Notification Descriptor.
+ * @li: Pointer to the Link Integrity Notification Descriptor.
*
* This function processes a Link Integrity FPIN event by logging a message.
**/
static void
-lpfc_els_rcv_fpin_li(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
+lpfc_els_rcv_fpin_li(struct lpfc_hba *phba, struct fc_fn_li_desc *li)
{
- struct fc_fn_li_desc *li = (struct fc_fn_li_desc *)tlv;
const char *li_evt_str;
u32 li_evt, cnt;
@@ -9980,14 +9975,13 @@ lpfc_els_rcv_fpin_li(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
/**
* lpfc_els_rcv_fpin_del - Process an FPIN Delivery Event.
* @phba: Pointer to hba object.
- * @tlv: Pointer to the Delivery Notification Descriptor TLV
+ * @del: Pointer to the Delivery Notification Descriptor TLV
*
* This function processes a Delivery FPIN event by logging a message.
**/
static void
-lpfc_els_rcv_fpin_del(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
+lpfc_els_rcv_fpin_del(struct lpfc_hba *phba, struct fc_fn_deli_desc *del)
{
- struct fc_fn_deli_desc *del = (struct fc_fn_deli_desc *)tlv;
const char *del_rsn_str;
u32 del_rsn;
__be32 *frame;
@@ -10018,14 +10012,14 @@ lpfc_els_rcv_fpin_del(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
/**
* lpfc_els_rcv_fpin_peer_cgn - Process a FPIN Peer Congestion Event.
* @phba: Pointer to hba object.
- * @tlv: Pointer to the Peer Congestion Notification Descriptor TLV
+ * @pc: Pointer to the Peer Congestion Notification Descriptor TLV
*
* This function processes a Peer Congestion FPIN event by logging a message.
**/
static void
-lpfc_els_rcv_fpin_peer_cgn(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
+lpfc_els_rcv_fpin_peer_cgn(struct lpfc_hba *phba,
+ struct fc_fn_peer_congn_desc *pc)
{
- struct fc_fn_peer_congn_desc *pc = (struct fc_fn_peer_congn_desc *)tlv;
const char *pc_evt_str;
u32 pc_evt, cnt;
@@ -10053,7 +10047,7 @@ lpfc_els_rcv_fpin_peer_cgn(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
/**
* lpfc_els_rcv_fpin_cgn - Process an FPIN Congestion notification
* @phba: Pointer to hba object.
- * @tlv: Pointer to the Congestion Notification Descriptor TLV
+ * @cgn: Pointer to the Congestion Notification Descriptor TLV
*
* This function processes an FPIN Congestion Notifiction. The notification
* could be an Alarm or Warning. This routine feeds that data into driver's
@@ -10062,10 +10056,9 @@ lpfc_els_rcv_fpin_peer_cgn(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
* to the upper layer or 0 to indicate don't deliver it.
**/
static int
-lpfc_els_rcv_fpin_cgn(struct lpfc_hba *phba, struct fc_tlv_desc *tlv)
+lpfc_els_rcv_fpin_cgn(struct lpfc_hba *phba, struct fc_fn_congn_desc *cgn)
{
struct lpfc_cgn_info *cp;
- struct fc_fn_congn_desc *cgn = (struct fc_fn_congn_desc *)tlv;
const char *cgn_evt_str;
u32 cgn_evt;
const char *cgn_sev_str;
@@ -10168,7 +10161,7 @@ lpfc_els_rcv_fpin(struct lpfc_vport *vport, void *p, u32 fpin_length)
{
struct lpfc_hba *phba = vport->phba;
struct fc_els_fpin *fpin = (struct fc_els_fpin *)p;
- struct fc_tlv_desc *tlv, *first_tlv, *current_tlv;
+ union fc_tlv_desc *tlv, *first_tlv, *current_tlv;
const char *dtag_nm;
int desc_cnt = 0, bytes_remain, cnt;
u32 dtag, deliver = 0;
@@ -10193,7 +10186,7 @@ lpfc_els_rcv_fpin(struct lpfc_vport *vport, void *p, u32 fpin_length)
return;
}
- tlv = (struct fc_tlv_desc *)&fpin->fpin_desc[0];
+ tlv = &fpin->fpin_desc[0];
first_tlv = tlv;
bytes_remain = fpin_length - offsetof(struct fc_els_fpin, fpin_desc);
bytes_remain = min_t(u32, bytes_remain, be32_to_cpu(fpin->desc_len));
@@ -10201,22 +10194,22 @@ lpfc_els_rcv_fpin(struct lpfc_vport *vport, void *p, u32 fpin_length)
/* process each descriptor separately */
while (bytes_remain >= FC_TLV_DESC_HDR_SZ &&
bytes_remain >= FC_TLV_DESC_SZ_FROM_LENGTH(tlv)) {
- dtag = be32_to_cpu(tlv->desc_tag);
+ dtag = be32_to_cpu(tlv->hdr.desc_tag);
switch (dtag) {
case ELS_DTAG_LNK_INTEGRITY:
- lpfc_els_rcv_fpin_li(phba, tlv);
+ lpfc_els_rcv_fpin_li(phba, &tlv->li);
deliver = 1;
break;
case ELS_DTAG_DELIVERY:
- lpfc_els_rcv_fpin_del(phba, tlv);
+ lpfc_els_rcv_fpin_del(phba, &tlv->deli);
deliver = 1;
break;
case ELS_DTAG_PEER_CONGEST:
- lpfc_els_rcv_fpin_peer_cgn(phba, tlv);
+ lpfc_els_rcv_fpin_peer_cgn(phba, &tlv->peer_congn);
deliver = 1;
break;
case ELS_DTAG_CONGESTION:
- deliver = lpfc_els_rcv_fpin_cgn(phba, tlv);
+ deliver = lpfc_els_rcv_fpin_cgn(phba, &tlv->congn);
break;
default:
dtag_nm = lpfc_get_tlv_dtag_nm(dtag);
@@ -10229,12 +10222,12 @@ lpfc_els_rcv_fpin(struct lpfc_vport *vport, void *p, u32 fpin_length)
return;
}
lpfc_cgn_update_stat(phba, dtag);
- cnt = be32_to_cpu(tlv->desc_len);
+ cnt = be32_to_cpu(tlv->hdr.desc_len);
/* Sanity check descriptor length. The desc_len value does not
* include space for the desc_tag and the desc_len fields.
*/
- len -= (cnt + sizeof(struct fc_tlv_desc));
+ len -= (cnt + sizeof(struct fc_tlv_desc_hdr));
if (len < 0) {
dtag_nm = lpfc_get_tlv_dtag_nm(dtag);
lpfc_printf_log(phba, KERN_WARNING, LOG_CGN_MGMT,
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 3a821afee9bc..f31fdc840191 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -743,13 +743,12 @@ fc_cn_stats_update(u16 event_type, struct fc_fpin_stats *stats)
*
*/
static void
-fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
+fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_fn_li_desc *li_desc)
{
u8 i;
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
- struct fc_fn_li_desc *li_desc = (struct fc_fn_li_desc *)tlv;
u16 event_type = be16_to_cpu(li_desc->event_type);
u64 wwpn;
@@ -792,12 +791,11 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
*/
static void
fc_fpin_delivery_stats_update(struct Scsi_Host *shost,
- struct fc_tlv_desc *tlv)
+ struct fc_fn_deli_desc *dn_desc)
{
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
- struct fc_fn_deli_desc *dn_desc = (struct fc_fn_deli_desc *)tlv;
u32 reason_code = be32_to_cpu(dn_desc->deli_reason_code);
rport = fc_find_rport_by_wwpn(shost,
@@ -823,13 +821,11 @@ fc_fpin_delivery_stats_update(struct Scsi_Host *shost,
*/
static void
fc_fpin_peer_congn_stats_update(struct Scsi_Host *shost,
- struct fc_tlv_desc *tlv)
+ struct fc_fn_peer_congn_desc *pc_desc)
{
u8 i;
struct fc_rport *rport = NULL;
struct fc_rport *attach_rport = NULL;
- struct fc_fn_peer_congn_desc *pc_desc =
- (struct fc_fn_peer_congn_desc *)tlv;
u16 event_type = be16_to_cpu(pc_desc->event_type);
u64 wwpn;
@@ -869,10 +865,9 @@ fc_fpin_peer_congn_stats_update(struct Scsi_Host *shost,
*/
static void
fc_fpin_congn_stats_update(struct Scsi_Host *shost,
- struct fc_tlv_desc *tlv)
+ struct fc_fn_congn_desc *congn)
{
struct fc_host_attrs *fc_host = shost_to_fc_host(shost);
- struct fc_fn_congn_desc *congn = (struct fc_fn_congn_desc *)tlv;
fc_cn_stats_update(be16_to_cpu(congn->event_type),
&fc_host->fpin_stats);
@@ -892,32 +887,32 @@ fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf,
u8 event_acknowledge)
{
struct fc_els_fpin *fpin = (struct fc_els_fpin *)fpin_buf;
- struct fc_tlv_desc *tlv;
+ union fc_tlv_desc *tlv;
u32 bytes_remain;
u32 dtag;
enum fc_host_event_code event_code =
event_acknowledge ? FCH_EVT_LINK_FPIN_ACK : FCH_EVT_LINK_FPIN;
/* Update Statistics */
- tlv = (struct fc_tlv_desc *)&fpin->fpin_desc[0];
+ tlv = &fpin->fpin_desc[0];
bytes_remain = fpin_len - offsetof(struct fc_els_fpin, fpin_desc);
bytes_remain = min_t(u32, bytes_remain, be32_to_cpu(fpin->desc_len));
while (bytes_remain >= FC_TLV_DESC_HDR_SZ &&
bytes_remain >= FC_TLV_DESC_SZ_FROM_LENGTH(tlv)) {
- dtag = be32_to_cpu(tlv->desc_tag);
+ dtag = be32_to_cpu(tlv->hdr.desc_tag);
switch (dtag) {
case ELS_DTAG_LNK_INTEGRITY:
- fc_fpin_li_stats_update(shost, tlv);
+ fc_fpin_li_stats_update(shost, &tlv->li);
break;
case ELS_DTAG_DELIVERY:
- fc_fpin_delivery_stats_update(shost, tlv);
+ fc_fpin_delivery_stats_update(shost, &tlv->deli);
break;
case ELS_DTAG_PEER_CONGEST:
- fc_fpin_peer_congn_stats_update(shost, tlv);
+ fc_fpin_peer_congn_stats_update(shost, &tlv->peer_congn);
break;
case ELS_DTAG_CONGESTION:
- fc_fpin_congn_stats_update(shost, tlv);
+ fc_fpin_congn_stats_update(shost, &tlv->congn);
}
bytes_remain -= FC_TLV_DESC_SZ_FROM_LENGTH(tlv);
diff --git a/include/uapi/scsi/fc/fc_els.h b/include/uapi/scsi/fc/fc_els.h
index 16782c360de3..3598dc553f4d 100644
--- a/include/uapi/scsi/fc/fc_els.h
+++ b/include/uapi/scsi/fc/fc_els.h
@@ -253,12 +253,12 @@ enum fc_ls_tlv_dtag {
/*
- * Generic Link Service TLV Descriptor format
+ * Generic Link Service TLV Descriptor header
*
* This structure, as it defines no payload, will also be referred to
* as the "tlv header" - which contains the tag and len fields.
*/
-struct fc_tlv_desc {
+struct fc_tlv_desc_hdr {
__be32 desc_tag; /* Notification Descriptor Tag */
__be32 desc_len; /* Length of Descriptor (in bytes).
* Size of descriptor excluding
@@ -267,36 +267,6 @@ struct fc_tlv_desc {
__u8 desc_value[]; /* Descriptor Value */
};
-/* Descriptor tag and len fields are considered the mandatory header
- * for a descriptor
- */
-#define FC_TLV_DESC_HDR_SZ sizeof(struct fc_tlv_desc)
-
-/*
- * Macro, used when initializing payloads, to return the descriptor length.
- * Length is size of descriptor minus the tag and len fields.
- */
-#define FC_TLV_DESC_LENGTH_FROM_SZ(desc) \
- (sizeof(desc) - FC_TLV_DESC_HDR_SZ)
-
-/* Macro, used on received payloads, to return the descriptor length */
-#define FC_TLV_DESC_SZ_FROM_LENGTH(tlv) \
- (__be32_to_cpu((tlv)->desc_len) + FC_TLV_DESC_HDR_SZ)
-
-/*
- * This helper is used to walk descriptors in a descriptor list.
- * Given the address of the current descriptor, which minimally contains a
- * tag and len field, calculate the address of the next descriptor based
- * on the len field.
- */
-static inline void *fc_tlv_next_desc(void *desc)
-{
- struct fc_tlv_desc *tlv = desc;
-
- return (desc + FC_TLV_DESC_SZ_FROM_LENGTH(tlv));
-}
-
-
/*
* Link Service Request Information Descriptor
*/
@@ -1094,19 +1064,6 @@ struct fc_fn_congn_desc {
__u8 resv[3]; /* reserved - must be zero */
};
-/*
- * ELS_FPIN - Fabric Performance Impact Notification
- */
-struct fc_els_fpin {
- __u8 fpin_cmd; /* command (0x16) */
- __u8 fpin_zero[3]; /* specified as zero - part of cmd */
- __be32 desc_len; /* Length of Descriptor List (in bytes).
- * Size of ELS excluding fpin_cmd,
- * fpin_zero and desc_len fields.
- */
- struct fc_tlv_desc fpin_desc[]; /* Descriptor list */
-};
-
/* Diagnostic Function Descriptor - FPIN Registration */
struct fc_df_desc_fpin_reg {
__be32 desc_tag; /* FPIN Registration (0x00030001) */
@@ -1125,33 +1082,6 @@ struct fc_df_desc_fpin_reg {
*/
};
-/*
- * ELS_RDF - Register Diagnostic Functions
- */
-struct fc_els_rdf {
- __u8 fpin_cmd; /* command (0x19) */
- __u8 fpin_zero[3]; /* specified as zero - part of cmd */
- __be32 desc_len; /* Length of Descriptor List (in bytes).
- * Size of ELS excluding fpin_cmd,
- * fpin_zero and desc_len fields.
- */
- struct fc_tlv_desc desc[]; /* Descriptor list */
-};
-
-/*
- * ELS RDF LS_ACC Response.
- */
-struct fc_els_rdf_resp {
- struct fc_els_ls_acc acc_hdr;
- __be32 desc_list_len; /* Length of response (in
- * bytes). Excludes acc_hdr
- * and desc_list_len fields.
- */
- struct fc_els_lsri_desc lsri;
- struct fc_tlv_desc desc[]; /* Supported Descriptor list */
-};
-
-
/*
* Diagnostic Capability Descriptors for EDC ELS
*/
@@ -1221,6 +1151,65 @@ struct fc_diag_cg_sig_desc {
struct fc_diag_cg_sig_freq rcv_signal_frequency;
};
+/*
+ * Generic Link Service TLV Descriptor format
+ *
+ * This structure, as it defines no payload, will also be referred to
+ * as the "tlv header" - which contains the tag and len fields.
+ */
+union fc_tlv_desc {
+ struct fc_tlv_desc_hdr hdr;
+ struct fc_els_lsri_desc lsri;
+ struct fc_fn_li_desc li;
+ struct fc_fn_deli_desc deli;
+ struct fc_fn_peer_congn_desc peer_congn;
+ struct fc_fn_congn_desc congn;
+ struct fc_df_desc_fpin_reg fpin_reg;
+ struct fc_diag_lnkflt_desc lnkflt;
+ struct fc_diag_cg_sig_desc cg_sig;
+};
+
+/* Descriptor tag and len fields are considered the mandatory header
+ * for a descriptor
+ */
+#define FC_TLV_DESC_HDR_SZ sizeof(struct fc_tlv_desc_hdr)
+
+/*
+ * Macro, used when initializing payloads, to return the descriptor length.
+ * Length is size of descriptor minus the tag and len fields.
+ */
+#define FC_TLV_DESC_LENGTH_FROM_SZ(desc) \
+ (sizeof(desc) - FC_TLV_DESC_HDR_SZ)
+
+/* Macro, used on received payloads, to return the descriptor length */
+#define FC_TLV_DESC_SZ_FROM_LENGTH(tlv) \
+ (__be32_to_cpu((tlv)->hdr.desc_len) + FC_TLV_DESC_HDR_SZ)
+
+/*
+ * This helper is used to walk descriptors in a descriptor list.
+ * Given the address of the current descriptor, which minimally contains a
+ * tag and len field, calculate the address of the next descriptor based
+ * on the len field.
+ */
+static inline union fc_tlv_desc *fc_tlv_next_desc(union fc_tlv_desc *desc)
+{
+ return (union fc_tlv_desc *)((__u8 *)desc + FC_TLV_DESC_SZ_FROM_LENGTH(desc));
+}
+
+
+/*
+ * ELS_FPIN - Fabric Performance Impact Notification
+ */
+struct fc_els_fpin {
+ __u8 fpin_cmd; /* command (0x16) */
+ __u8 fpin_zero[3]; /* specified as zero - part of cmd */
+ __be32 desc_len; /* Length of Descriptor List (in bytes).
+ * Size of ELS excluding fpin_cmd,
+ * fpin_zero and desc_len fields.
+ */
+ union fc_tlv_desc fpin_desc[]; /* Descriptor list */
+};
+
/*
* ELS_EDC - Exchange Diagnostic Capabilities
*/
@@ -1231,10 +1220,37 @@ struct fc_els_edc {
* Size of ELS excluding edc_cmd,
* edc_zero and desc_len fields.
*/
- struct fc_tlv_desc desc[];
+ union fc_tlv_desc desc[];
/* Diagnostic Descriptor list */
};
+/*
+ * ELS_RDF - Register Diagnostic Functions
+ */
+struct fc_els_rdf {
+ __u8 fpin_cmd; /* command (0x19) */
+ __u8 fpin_zero[3]; /* specified as zero - part of cmd */
+ __be32 desc_len; /* Length of Descriptor List (in bytes).
+ * Size of ELS excluding fpin_cmd,
+ * fpin_zero and desc_len fields.
+ */
+ union fc_tlv_desc desc[]; /* Descriptor list */
+};
+
+/*
+ * ELS RDF LS_ACC Response.
+ */
+struct fc_els_rdf_resp {
+ struct fc_els_ls_acc acc_hdr;
+ __be32 desc_list_len; /* Length of response (in
+ * bytes). Excludes acc_hdr
+ * and desc_list_len fields.
+ */
+ struct fc_els_lsri_desc lsri;
+ union fc_tlv_desc desc[]; /* Supported Descriptor list */
+};
+
+
/*
* ELS EDC LS_ACC Response.
*/
@@ -1245,9 +1261,8 @@ struct fc_els_edc_resp {
* and desc_list_len fields.
*/
struct fc_els_lsri_desc lsri;
- struct fc_tlv_desc desc[];
+ union fc_tlv_desc desc[];
/* Supported Diagnostic Descriptor list */
};
-
#endif /* _FC_ELS_H_ */
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v10 01/11] fc_els: use 'union fc_tlv_desc'
2025-09-26 0:01 ` [PATCH v10 01/11] fc_els: use 'union fc_tlv_desc' John Meneghini
@ 2025-09-29 17:44 ` Justin Tee
2025-09-30 10:15 ` John Meneghini
0 siblings, 1 reply; 21+ messages in thread
From: Justin Tee @ 2025-09-29 17:44 UTC (permalink / raw)
To: John Meneghini
Cc: hare, kbusch, martin.petersen, linux-nvme, linux-scsi, bgurney,
axboe, emilne, gustavoars, hch, james.smart, Justin Tee, kees,
linux-hardening, njavali, sagi
Hi John,
Which branch is this patch based on? include/uapi/scsi/fc/fc_els.h
does not apply cleanly on 6.18/scsi-queue.
Presumably, is this patch based on a branch without?
44b6169ada7f scsi: fc: Avoid -Wflex-array-member-not-at-end warnings
Regards,
Justin
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v10 01/11] fc_els: use 'union fc_tlv_desc'
2025-09-29 17:44 ` Justin Tee
@ 2025-09-30 10:15 ` John Meneghini
0 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-30 10:15 UTC (permalink / raw)
To: Justin Tee
Cc: hare, kbusch, martin.petersen, linux-nvme, linux-scsi, bgurney,
axboe, emilne, gustavoars, hch, james.smart, Justin Tee, kees,
linux-hardening, njavali, sagi
On 9/29/25 1:44 PM, Justin Tee wrote:> Hi John,
>
> Which branch is this patch based on? include/uapi/scsi/fc/fc_els.h
> does not apply cleanly on 6.18/scsi-queue.
It's in the patch cover letter. I've re-based V11 onto nvme-6.18.
> Presumably, is this patch based on a branch without?
> 44b6169ada7f scsi: fc: Avoid -Wflex-array-member-not-at-end warnings
Yes
> Regards,
> Justin
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v10 02/11] nvme: add NVME_CTRL_MARGINAL flag
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
2025-09-26 0:01 ` [PATCH v10 01/11] fc_els: use 'union fc_tlv_desc' John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-26 0:01 ` [PATCH v10 03/11] nvme-fc: marginal path handling John Meneghini
` (8 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
From: Bryan Gurney <bgurney@redhat.com>
Add a new controller flag, NVME_CTRL_MARGINAL, to help multipath I/O
policies to react to a path that is set to a "marginal" state.
The flag is cleared on controller reset, which is often the case when
faulty cabling or transceiver hardware is replaced.
Signed-off-by: Bryan Gurney <bgurney@redhat.com>
---
drivers/nvme/host/core.c | 1 +
drivers/nvme/host/nvme.h | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index edfe7a267a4b..d505acd0ebf7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -5074,6 +5074,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
WRITE_ONCE(ctrl->state, NVME_CTRL_NEW);
ctrl->passthru_err_log_enabled = false;
clear_bit(NVME_CTRL_FAILFAST_EXPIRED, &ctrl->flags);
+ clear_bit(NVME_CTRL_MARGINAL, &ctrl->flags);
spin_lock_init(&ctrl->lock);
mutex_init(&ctrl->namespaces_lock);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index cfd2b5b90b91..d71e6668f11c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -275,6 +275,7 @@ enum nvme_ctrl_flags {
NVME_CTRL_SKIP_ID_CNS_CS = 4,
NVME_CTRL_DIRTY_CAPABILITY = 5,
NVME_CTRL_FROZEN = 6,
+ NVME_CTRL_MARGINAL = 7,
};
struct nvme_ctrl {
@@ -417,6 +418,11 @@ static inline enum nvme_ctrl_state nvme_ctrl_state(struct nvme_ctrl *ctrl)
return READ_ONCE(ctrl->state);
}
+static inline bool nvme_ctrl_is_marginal(struct nvme_ctrl *ctrl)
+{
+ return test_bit(NVME_CTRL_MARGINAL, &ctrl->flags);
+}
+
enum nvme_iopolicy {
NVME_IOPOLICY_NUMA,
NVME_IOPOLICY_RR,
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v10 03/11] nvme-fc: marginal path handling
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
2025-09-26 0:01 ` [PATCH v10 01/11] fc_els: use 'union fc_tlv_desc' John Meneghini
2025-09-26 0:01 ` [PATCH v10 02/11] nvme: add NVME_CTRL_MARGINAL flag John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-10-03 10:15 ` Christoph Hellwig
2025-09-26 0:01 ` [PATCH v10 04/11] nvme: sysfs: emit the marginal path state in show_state() John Meneghini
` (7 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
From: Hannes Reinecke <hare@kernel.org>
FPIN LI (link integrity) messages are received when the attached
fabric detects hardware errors. In response to these messages I/O
should be directed away from the affected ports, and only used
if the 'optimized' paths are unavailable.
To handle this a new controller flag 'NVME_CTRL_MARGINAL' is added
which will cause the multipath scheduler to skip these paths when
checking for 'optimized' paths. They are, however, still eligible
for non-optimized path selected. The flag is cleared upon reset as then the
faulty hardware might be replaced.
Signed-off-by: Hannes Reinecke <hare@kernel.org>
Tested-by: Bryan Gurney <bgurney@redhat.com>
Reviewed-by: John Meneghini <jmeneghi@redhat.com>
Tested-by: Muneendra Kumar <muneendra.kumar@broadcom.com>
---
drivers/nvme/host/fc.c | 4 ++++
drivers/nvme/host/multipath.c | 17 +++++++++++------
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 03987f497a5b..5091927c2176 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -786,6 +786,10 @@ nvme_fc_ctrl_connectivity_loss(struct nvme_fc_ctrl *ctrl)
"Reconnect", ctrl->cnum);
set_bit(ASSOC_FAILED, &ctrl->flags);
+
+ /* clear 'marginal' flag as controller will be reset */
+ clear_bit(NVME_CTRL_MARGINAL, &ctrl->flags);
+
nvme_reset_ctrl(&ctrl->ctrl);
}
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 3da980dc60d9..c042a9a11ce3 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -324,11 +324,14 @@ static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node)
switch (ns->ana_state) {
case NVME_ANA_OPTIMIZED:
- if (distance < found_distance) {
- found_distance = distance;
- found = ns;
+ if (!nvme_ctrl_is_marginal(ns->ctrl)) {
+ if (distance < found_distance) {
+ found_distance = distance;
+ found = ns;
+ }
+ break;
}
- break;
+ fallthrough;
case NVME_ANA_NONOPTIMIZED:
if (distance < fallback_distance) {
fallback_distance = distance;
@@ -381,7 +384,8 @@ static struct nvme_ns *nvme_round_robin_path(struct nvme_ns_head *head)
if (ns->ana_state == NVME_ANA_OPTIMIZED) {
found = ns;
- goto out;
+ if (!nvme_ctrl_is_marginal(ns->ctrl))
+ goto out;
}
if (ns->ana_state == NVME_ANA_NONOPTIMIZED)
found = ns;
@@ -445,7 +449,8 @@ static struct nvme_ns *nvme_queue_depth_path(struct nvme_ns_head *head)
static inline bool nvme_path_is_optimized(struct nvme_ns *ns)
{
return nvme_ctrl_state(ns->ctrl) == NVME_CTRL_LIVE &&
- ns->ana_state == NVME_ANA_OPTIMIZED;
+ ns->ana_state == NVME_ANA_OPTIMIZED &&
+ !nvme_ctrl_is_marginal(ns->ctrl);
}
static struct nvme_ns *nvme_numa_path(struct nvme_ns_head *head)
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v10 03/11] nvme-fc: marginal path handling
2025-09-26 0:01 ` [PATCH v10 03/11] nvme-fc: marginal path handling John Meneghini
@ 2025-10-03 10:15 ` Christoph Hellwig
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2025-10-03 10:15 UTC (permalink / raw)
To: John Meneghini
Cc: hare, kbusch, martin.petersen, linux-nvme, linux-scsi, bgurney,
axboe, emilne, gustavoars, hch, james.smart, kees,
linux-hardening, njavali, sagi
Please don't hide core changes in patches marked nvme-fc.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v10 04/11] nvme: sysfs: emit the marginal path state in show_state()
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
` (2 preceding siblings ...)
2025-09-26 0:01 ` [PATCH v10 03/11] nvme-fc: marginal path handling John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-26 0:01 ` [PATCH v10 05/11] nvme-multipath: queue-depth support for marginal paths John Meneghini
` (6 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
From: Bryan Gurney <bgurney@redhat.com>
If a controller has received a link integrity or congestion event, and
has the NVME_CTRL_MARGINAL flag set, emit "marginal" in the state
instead of "live", to identify the marginal paths.
Co-developed-by: John Meneghini <jmeneghi@redhat.com>
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Tested-by: Muneendra Kumar <muneendra.kumar@broadcom.com>
Signed-off-by: Bryan Gurney <bgurney@redhat.com>
---
drivers/nvme/host/sysfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 29430949ce2f..4a6135c2f9cb 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -430,7 +430,9 @@ static ssize_t nvme_sysfs_show_state(struct device *dev,
};
if (state < ARRAY_SIZE(state_name) && state_name[state])
- return sysfs_emit(buf, "%s\n", state_name[state]);
+ return sysfs_emit(buf, "%s\n",
+ (nvme_ctrl_is_marginal(ctrl)) ? "marginal" :
+ state_name[state]);
return sysfs_emit(buf, "unknown state\n");
}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v10 05/11] nvme-multipath: queue-depth support for marginal paths
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
` (3 preceding siblings ...)
2025-09-26 0:01 ` [PATCH v10 04/11] nvme: sysfs: emit the marginal path state in show_state() John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-26 0:01 ` [PATCH v10 06/11] nvme-fc: add nvme_fc_modify_rport_fpin_state() John Meneghini
` (5 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
Exclude marginal paths from queue-depth io policy. In the case where all
paths are marginal and no optimized or non-optimized path is found, we
fall back to __nvme_find_path which selects the best marginal path.
Tested-by: Bryan Gurney <bgurney@redhat.com>
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
drivers/nvme/host/multipath.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index c042a9a11ce3..38e40dd88e52 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -420,6 +420,9 @@ static struct nvme_ns *nvme_queue_depth_path(struct nvme_ns_head *head)
if (nvme_path_is_disabled(ns))
continue;
+ if (nvme_ctrl_is_marginal(ns->ctrl))
+ continue;
+
depth = atomic_read(&ns->ctrl->nr_active);
switch (ns->ana_state) {
@@ -443,7 +446,9 @@ static struct nvme_ns *nvme_queue_depth_path(struct nvme_ns_head *head)
return best_opt;
}
- return best_opt ? best_opt : best_nonopt;
+ best_opt = (best_opt) ? best_opt : best_nonopt;
+
+ return best_opt ? best_opt : __nvme_find_path(head, numa_node_id());
}
static inline bool nvme_path_is_optimized(struct nvme_ns *ns)
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v10 06/11] nvme-fc: add nvme_fc_modify_rport_fpin_state()
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
` (4 preceding siblings ...)
2025-09-26 0:01 ` [PATCH v10 05/11] nvme-multipath: queue-depth support for marginal paths John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-26 0:01 ` [PATCH v10 07/11] scsi: scsi_transport_fc: add fc_host_fpin_set_nvme_rport_marginal() John Meneghini
` (4 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
Add nvme_fc_modify_rport_fpin_state() and supporting functions. This
function is called by the SCSI FC transport and driver layer to set or
clear the 'marginal' path status for a specific rport.
Co-developed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
Tested-by: Bryan Gurney <bgurney@redhat.com>
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
drivers/nvme/host/fc.c | 76 ++++++++++++++++++++++++++++++++++
include/linux/nvme-fc-driver.h | 2 +
2 files changed, 78 insertions(+)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 5091927c2176..87bfe34b4d52 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3730,6 +3730,82 @@ static struct nvmf_transport_ops nvme_fc_transport = {
.create_ctrl = nvme_fc_create_ctrl,
};
+static struct nvme_fc_rport *nvme_fc_rport_from_wwpn(struct nvme_fc_lport *lport,
+ u64 rport_wwpn)
+{
+ struct nvme_fc_rport *rport;
+ unsigned long flags;
+
+ spin_lock_irqsave(&nvme_fc_lock, flags);
+ list_for_each_entry(rport, &lport->endp_list, endp_list) {
+ if (!nvme_fc_rport_get(rport))
+ continue;
+ if (rport->remoteport.port_name == rport_wwpn &&
+ rport->remoteport.port_role & FC_PORT_ROLE_NVME_TARGET) {
+ spin_unlock_irqrestore(&nvme_fc_lock, flags);
+ return rport;
+ }
+ nvme_fc_rport_put(rport);
+ }
+ spin_unlock_irqrestore(&nvme_fc_lock, flags);
+ return NULL;
+}
+
+static struct nvme_fc_lport *
+nvme_fc_lport_from_wwpn(u64 wwpn)
+{
+ struct nvme_fc_lport *lport;
+ unsigned long flags;
+
+ spin_lock_irqsave(&nvme_fc_lock, flags);
+ list_for_each_entry(lport, &nvme_fc_lport_list, port_list) {
+ if (lport->localport.port_name == wwpn &&
+ lport->localport.port_state == FC_OBJSTATE_ONLINE) {
+ if (nvme_fc_lport_get(lport)) {
+ spin_unlock_irqrestore(&nvme_fc_lock, flags);
+ return lport;
+ }
+ }
+ }
+ spin_unlock_irqrestore(&nvme_fc_lock, flags);
+ return NULL;
+}
+
+static void
+nvme_fc_fpin_set_state(struct nvme_fc_lport *lport, u64 wwpn, bool marginal)
+{
+ struct nvme_fc_rport *rport;
+ struct nvme_fc_ctrl *ctrl;
+
+ rport = nvme_fc_rport_from_wwpn(lport, wwpn);
+ if (!rport)
+ return;
+
+ spin_lock_irq(&rport->lock);
+ list_for_each_entry(ctrl, &rport->ctrl_list, ctrl_list) {
+ if (marginal)
+ set_bit(NVME_CTRL_MARGINAL, &ctrl->ctrl.flags);
+ else
+ clear_bit(NVME_CTRL_MARGINAL, &ctrl->ctrl.flags);
+ }
+ spin_unlock_irq(&rport->lock);
+ nvme_fc_rport_put(rport);
+}
+
+void
+nvme_fc_modify_rport_fpin_state(u64 local_wwpn, u64 remote_wwpn, bool marginal)
+{
+ struct nvme_fc_lport *lport;
+
+ lport = nvme_fc_lport_from_wwpn(local_wwpn);
+ if (!lport)
+ return;
+
+ nvme_fc_fpin_set_state(lport, remote_wwpn, marginal);
+ nvme_fc_lport_put(lport);
+}
+EXPORT_SYMBOL_GPL(nvme_fc_modify_rport_fpin_state);
+
/* Arbitrary successive failures max. With lots of subsystems could be high */
#define DISCOVERY_MAX_FAIL 20
diff --git a/include/linux/nvme-fc-driver.h b/include/linux/nvme-fc-driver.h
index 9f6acadfe0c8..b026e6312f85 100644
--- a/include/linux/nvme-fc-driver.h
+++ b/include/linux/nvme-fc-driver.h
@@ -536,6 +536,8 @@ void nvme_fc_rescan_remoteport(struct nvme_fc_remote_port *remoteport);
int nvme_fc_set_remoteport_devloss(struct nvme_fc_remote_port *remoteport,
u32 dev_loss_tmo);
+void nvme_fc_modify_rport_fpin_state(u64 local_wwpn, u64 remote_wwpn, bool marginal);
+
/*
* Routine called to pass a NVME-FC LS request, received by the lldd,
* to the nvme-fc transport.
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v10 07/11] scsi: scsi_transport_fc: add fc_host_fpin_set_nvme_rport_marginal()
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
` (5 preceding siblings ...)
2025-09-26 0:01 ` [PATCH v10 06/11] nvme-fc: add nvme_fc_modify_rport_fpin_state() John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-29 17:45 ` Justin Tee
2025-09-26 0:01 ` [PATCH v10 08/11] scsi: lpfc: enable FPIN notification for NVMe John Meneghini
` (3 subsequent siblings)
10 siblings, 1 reply; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
Add fc_host_fpin_set_nvme_rport_marginal() function to
evaluate the FPIN LI TLV information and set the 'marginal'
path status for all affected nvme rports.
Co-developed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
Tested-by: Bryan Gurney <bgurney@redhat.com>
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
drivers/scsi/scsi_transport_fc.c | 81 ++++++++++++++++++++++++++++++++
include/scsi/scsi_transport_fc.h | 1 +
2 files changed, 82 insertions(+)
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index f31fdc840191..4dc03cbaf3e2 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -25,6 +25,10 @@
#include <uapi/scsi/fc/fc_els.h>
#include "scsi_priv.h"
+#if (IS_ENABLED(CONFIG_NVME_FC))
+void nvme_fc_modify_rport_fpin_state(u64 local_wwpn, u64 remote_wwpn, bool marginal);
+#endif
+
static int fc_queue_work(struct Scsi_Host *, struct work_struct *);
static void fc_vport_sched_delete(struct work_struct *work);
static int fc_vport_setup(struct Scsi_Host *shost, int channel,
@@ -873,6 +877,83 @@ fc_fpin_congn_stats_update(struct Scsi_Host *shost,
&fc_host->fpin_stats);
}
+/**
+ * fc_host_fpin_set_nvme_rport_marginal
+ * - Set FC_PORTSTATE_MARGINAL for nvme rports in FPIN
+ * @shost: host the FPIN was received on
+ * @fpin_len: length of FPIN payload, in bytes
+ * @fpin_buf: pointer to FPIN payload
+ *
+ * This function processes a received FPIN and sets FC_PORTSTATE_MARGINAL on
+ * all remote ports that have FC_PORT_ROLE_NVME_TARGET set identified in the
+ * FPIN descriptors.
+ *
+ * Notes:
+ * This routine assumes no locks are held on entry.
+ */
+void
+fc_host_fpin_set_nvme_rport_marginal(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf)
+{
+ struct fc_els_fpin *fpin = (struct fc_els_fpin *)fpin_buf;
+ struct fc_rport *rport;
+ union fc_tlv_desc *tlv;
+ u64 local_wwpn = fc_host_port_name(shost);
+ u64 wwpn, attached_wwpn;
+ u32 bytes_remain;
+ u32 dtag;
+ u8 i;
+ unsigned long flags;
+
+ /* Parse FPIN descriptors */
+ tlv = &fpin->fpin_desc[0];
+ bytes_remain = fpin_len - offsetof(struct fc_els_fpin, fpin_desc);
+ bytes_remain = min_t(u32, bytes_remain, be32_to_cpu(fpin->desc_len));
+
+ while (bytes_remain >= FC_TLV_DESC_HDR_SZ &&
+ bytes_remain >= FC_TLV_DESC_SZ_FROM_LENGTH(tlv)) {
+ dtag = be32_to_cpu(tlv->hdr.desc_tag);
+ switch (dtag) {
+ case ELS_DTAG_LNK_INTEGRITY:
+ struct fc_fn_li_desc *li_desc = &tlv->li;
+
+ attached_wwpn = be64_to_cpu(li_desc->attached_wwpn);
+
+ /* Set marginal state for WWPNs in pname_list */
+ if (be32_to_cpu(li_desc->pname_count) > 0) {
+ for (i = 0; i < be32_to_cpu(li_desc->pname_count); i++) {
+ wwpn = be64_to_cpu(li_desc->pname_list[i]);
+ if (wwpn == attached_wwpn)
+ continue;
+
+ rport = fc_find_rport_by_wwpn(shost, wwpn);
+ if (!rport)
+ continue;
+
+ spin_lock_irqsave(shost->host_lock, flags);
+
+ if (rport->port_state == FC_PORTSTATE_ONLINE &&
+ rport->roles & FC_PORT_ROLE_NVME_TARGET) {
+ rport->port_state = FC_PORTSTATE_MARGINAL;
+ spin_unlock_irqrestore(shost->host_lock, flags);
+#if (IS_ENABLED(CONFIG_NVME_FC))
+ nvme_fc_modify_rport_fpin_state(local_wwpn,
+ wwpn, true);
+#endif
+ } else
+ spin_unlock_irqrestore(shost->host_lock, flags);
+
+ }
+ }
+ break;
+ default:
+ break;
+ }
+ bytes_remain -= FC_TLV_DESC_SZ_FROM_LENGTH(tlv);
+ tlv = fc_tlv_next_desc(tlv);
+ }
+}
+EXPORT_SYMBOL(fc_host_fpin_set_nvme_rport_marginal);
+
/**
* fc_host_fpin_rcv - routine to process a received FPIN.
* @shost: host the FPIN was received on
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index b908aacfef48..e8d46b71bada 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -852,6 +852,7 @@ void fc_host_post_fc_event(struct Scsi_Host *shost, u32 event_number,
*/
void fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf,
u8 event_acknowledge);
+void fc_host_fpin_set_nvme_rport_marginal(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf);
struct fc_vport *fc_vport_create(struct Scsi_Host *shost, int channel,
struct fc_vport_identifiers *);
int fc_vport_terminate(struct fc_vport *vport);
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v10 07/11] scsi: scsi_transport_fc: add fc_host_fpin_set_nvme_rport_marginal()
2025-09-26 0:01 ` [PATCH v10 07/11] scsi: scsi_transport_fc: add fc_host_fpin_set_nvme_rport_marginal() John Meneghini
@ 2025-09-29 17:45 ` Justin Tee
2025-09-30 10:17 ` John Meneghini
0 siblings, 1 reply; 21+ messages in thread
From: Justin Tee @ 2025-09-29 17:45 UTC (permalink / raw)
To: John Meneghini
Cc: hare, kbusch, martin.petersen, linux-nvme, linux-scsi, bgurney,
axboe, emilne, gustavoars, hch, Justin Tee, james.smart, kees,
linux-hardening, njavali, sagi
Hi John,
> + u64 local_wwpn = fc_host_port_name(shost);
If CONFIG_NVME_FC is not enabled, then the u64 local_wwpn variables
aren’t used anywhere.
drivers/scsi/scsi_transport_fc.c: In function
‘fc_host_fpin_set_nvme_rport_marginal’:
drivers/scsi/scsi_transport_fc.c:900:13: warning: unused variable
‘local_wwpn’ [-Wunused-variable]
900 | u64 local_wwpn = fc_host_port_name(shost);
| ^~~~~~~~~~
drivers/scsi/scsi_transport_fc.c: In function ‘fc_rport_set_marginal_state’:
drivers/scsi/scsi_transport_fc.c:1309:13: warning: unused variable
‘local_wwpn’ [-Wunused-variable]
1309 | u64 local_wwpn = fc_host_port_name(shost);
| ^~~~~~~~~~
Perhaps we need to ifdef the u64 local_wwpn declarations in both
fc_host_fpin_set_nvme_rport_marginal and fc_rport_set_marginal_state
routines too?
#if (IS_ENABLED(CONFIG_NVME_FC))
u64 local_wwpn = fc_host_port_name(shost);
#endif
Regards,
Justin
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v10 07/11] scsi: scsi_transport_fc: add fc_host_fpin_set_nvme_rport_marginal()
2025-09-29 17:45 ` Justin Tee
@ 2025-09-30 10:17 ` John Meneghini
0 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-30 10:17 UTC (permalink / raw)
To: Justin Tee
Cc: hare, kbusch, martin.petersen, linux-nvme, linux-scsi, bgurney,
axboe, emilne, gustavoars, hch, Justin Tee, james.smart, kees,
linux-hardening, njavali, sagi
On 9/29/25 1:45 PM, Justin Tee wrote:
> Hi John,
>
>> + u64 local_wwpn = fc_host_port_name(shost);
> If CONFIG_NVME_FC is not enabled, then the u64 local_wwpn variables
> aren’t used anywhere.
Oops. Thanks I will fix this in my V11 patch series.
/John
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v10 08/11] scsi: lpfc: enable FPIN notification for NVMe
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
` (6 preceding siblings ...)
2025-09-26 0:01 ` [PATCH v10 07/11] scsi: scsi_transport_fc: add fc_host_fpin_set_nvme_rport_marginal() John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-26 0:01 ` [PATCH v10 09/11] scsi: qla2xxx: " John Meneghini
` (2 subsequent siblings)
10 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
Call fc_host_fpin_set_nvme_rport_marginal() to enable FPIN notifications
for NVMe.
Co-developed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
Tested-by: Bryan Gurney <bgurney@redhat.com>
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
drivers/scsi/lpfc/lpfc_els.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c
index 1511cabac636..bfd9ba4552f3 100644
--- a/drivers/scsi/lpfc/lpfc_els.c
+++ b/drivers/scsi/lpfc/lpfc_els.c
@@ -10256,9 +10256,14 @@ lpfc_els_rcv_fpin(struct lpfc_vport *vport, void *p, u32 fpin_length)
fpin_length += sizeof(struct fc_els_fpin); /* the entire FPIN */
/* Send every descriptor individually to the upper layer */
- if (deliver)
+ if (deliver) {
fc_host_fpin_rcv(lpfc_shost_from_vport(vport),
fpin_length, (char *)fpin, 0);
+ if (vport->cfg_enable_fc4_type & LPFC_ENABLE_NVME) {
+ fc_host_fpin_set_nvme_rport_marginal(lpfc_shost_from_vport(vport),
+ fpin_length, (char *)fpin);
+ }
+ }
desc_cnt++;
}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v10 09/11] scsi: qla2xxx: enable FPIN notification for NVMe
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
` (7 preceding siblings ...)
2025-09-26 0:01 ` [PATCH v10 08/11] scsi: lpfc: enable FPIN notification for NVMe John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-26 0:01 ` [PATCH v10 10/11] scsi: scsi_transport_fc: user support for clearing NVME_CTRL_MARGINAL John Meneghini
2025-09-26 0:02 ` [PATCH v10 11/11] scsi: qla2xxx: Fix 2 memcpy field-spanning write issue John Meneghini
10 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
Call fc_host_fpin_set_nvme_rport_marginal() to enable FPIN notifications
for NVMe.
Co-developed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
Tested-by: Bryan Gurney <bgurney@redhat.com>
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
drivers/scsi/qla2xxx/qla_isr.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index c4c6b5c6658c..8ff8781dae47 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -46,6 +46,7 @@ qla27xx_process_purex_fpin(struct scsi_qla_host *vha, struct purex_item *item)
pkt, pkt_size);
fc_host_fpin_rcv(vha->host, pkt_size, (char *)pkt, 0);
+ fc_host_fpin_set_nvme_rport_marginal(vha->host, pkt_size, (char *)pkt);
}
const char *const port_state_str[] = {
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v10 10/11] scsi: scsi_transport_fc: user support for clearing NVME_CTRL_MARGINAL
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
` (8 preceding siblings ...)
2025-09-26 0:01 ` [PATCH v10 09/11] scsi: qla2xxx: " John Meneghini
@ 2025-09-26 0:01 ` John Meneghini
2025-09-26 0:02 ` [PATCH v10 11/11] scsi: qla2xxx: Fix 2 memcpy field-spanning write issue John Meneghini
10 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:01 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
Refactor and fc_rport_set_marginal_state smp safe by holding
`shost->host_lock` around all `rport->port_state` accesses.
Call nvme_fc_modify_rport_fpin_state() when FC_PORTSTATE_MARGINAL is set
or cleared. This allows the user to quickly set or clear the
NVME_CTRL_MARGINAL state from sysfs.
E.g.:
echo "Marginal" > /sys/class/fc_remote_ports/rport-13:0-5/port_state
echo "Online" > /sys/class/fc_remote_ports/rport-13:0-5/port_state
Note: nvme_fc_modify_rport_fpin_state() will only affect rports that
have FC_PORT_ROLE_NVME_TARGET set.
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
---
drivers/scsi/scsi_transport_fc.c | 46 +++++++++++++++++++++++++-------
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 4dc03cbaf3e2..811530037a1e 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -1305,34 +1305,62 @@ static ssize_t fc_rport_set_marginal_state(struct device *dev,
const char *buf, size_t count)
{
struct fc_rport *rport = transport_class_to_rport(dev);
+ struct Scsi_Host *shost = rport_to_shost(rport);
+ u64 local_wwpn = fc_host_port_name(shost);
enum fc_port_state port_state;
int ret = 0;
+ unsigned long flags;
ret = get_fc_port_state_match(buf, &port_state);
if (ret)
return -EINVAL;
- if (port_state == FC_PORTSTATE_MARGINAL) {
+
+ spin_lock_irqsave(shost->host_lock, flags);
+
+ switch (port_state) {
+ case FC_PORTSTATE_MARGINAL:
/*
* Change the state to Marginal only if the
* current rport state is Online
* Allow only Online->Marginal
*/
- if (rport->port_state == FC_PORTSTATE_ONLINE)
+ if (rport->port_state == FC_PORTSTATE_ONLINE) {
rport->port_state = port_state;
- else if (port_state != rport->port_state)
- return -EINVAL;
- } else if (port_state == FC_PORTSTATE_ONLINE) {
+ spin_unlock_irqrestore(shost->host_lock, flags);
+#if (IS_ENABLED(CONFIG_NVME_FC))
+ nvme_fc_modify_rport_fpin_state(local_wwpn,
+ rport->port_name, true);
+#endif
+ return count;
+ }
+ break;
+
+ case FC_PORTSTATE_ONLINE:
/*
* Change the state to Online only if the
* current rport state is Marginal
* Allow only Marginal->Online
*/
- if (rport->port_state == FC_PORTSTATE_MARGINAL)
+ if (rport->port_state == FC_PORTSTATE_MARGINAL) {
rport->port_state = port_state;
- else if (port_state != rport->port_state)
- return -EINVAL;
- } else
+ spin_unlock_irqrestore(shost->host_lock, flags);
+#if (IS_ENABLED(CONFIG_NVME_FC))
+ nvme_fc_modify_rport_fpin_state(local_wwpn,
+ rport->port_name, false);
+#endif
+ return count;
+ }
+ break;
+ default:
+ break;
+ }
+
+ if (port_state != rport->port_state) {
+ spin_unlock_irqrestore(shost->host_lock, flags);
return -EINVAL;
+ }
+
+ spin_unlock_irqrestore(shost->host_lock, flags);
return count;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v10 11/11] scsi: qla2xxx: Fix 2 memcpy field-spanning write issue
2025-09-26 0:01 [PATCH v10 00/11] nvme-fc: FPIN link integrity handling John Meneghini
` (9 preceding siblings ...)
2025-09-26 0:01 ` [PATCH v10 10/11] scsi: scsi_transport_fc: user support for clearing NVME_CTRL_MARGINAL John Meneghini
@ 2025-09-26 0:02 ` John Meneghini
2025-09-26 9:00 ` Gustavo A. R. Silva
10 siblings, 1 reply; 21+ messages in thread
From: John Meneghini @ 2025-09-26 0:02 UTC (permalink / raw)
To: hare, kbusch, martin.petersen, linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, jmeneghi,
kees, linux-hardening, njavali, sagi
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
purex_item.iocb is defined as a 64-element u8 array, but 64 is the
minimum size and it can be allocated larger. This makes it a standard
empty flex array.
This was motivated by field-spanning write warnings during FPIN testing.
> kernel: memcpy: detected field-spanning write (size 60) of single
> field "((uint8_t *)fpin_pkt + buffer_copy_offset)"
> at drivers/scsi/qla2xxx/qla_isr.c:1221 (size 44)
I removed the outer wrapper from the iocb flex array, so that it can be
linked to `purex_item.size` with `__counted_by`.
These changes remove the default minimum 64-byte allocation, requiring
further changes.
In `struct scsi_qla_host` the embedded `default_item` is now followed
by `__default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE]` to reserve space
that will be used as `default_item.iocb`. This is wrapped using the
`TRAILING_OVERLAP()` macro helper, which effectively creates a union
between flexible-array member `default_item.iocb` and
`__default_item_iocb`.
Since `struct pure_item` now contains a flexible-array member, the
helper must be placed at the end of `struct scsi_qla_host` to prevent
a `-Wflex-array-member-not-at-end` warning.
`qla24xx_alloc_purex_item()` is adjusted to no longer expect the
default minimum size to be part of `sizeof(struct purex_item)`,
the entire flexible array size is added to the structure size for
allocation.
This also slightly changes the layout of the purex_item struct, as
2-bytes of padding are added between `size` and `iocb`. The resulting
size is the same, but iocb is shifted 2-bytes (the original `purex_item`
structure was padded at the end, after the 64-byte defined array size).
I don't think this is a problem.
In qla_os.c:qla24xx_process_purex_rdp()
To avoid a null pointer dereference the vha->default_item should be set
to 0 last if the item pointer passed to the function matches. Also use
a local variable to avoid multiple de-referencing of the item.
Tested-by: Bryan Gurney <bgurney@redhat.com>
Co-developed-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
drivers/scsi/qla2xxx/qla_def.h | 10 ++++++----
drivers/scsi/qla2xxx/qla_isr.c | 17 ++++++++---------
drivers/scsi/qla2xxx/qla_nvme.c | 2 +-
drivers/scsi/qla2xxx/qla_os.c | 9 ++++++---
4 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
index cb95b7b12051..604e66bead1e 100644
--- a/drivers/scsi/qla2xxx/qla_def.h
+++ b/drivers/scsi/qla2xxx/qla_def.h
@@ -4890,9 +4890,7 @@ struct purex_item {
struct purex_item *pkt);
atomic_t in_use;
uint16_t size;
- struct {
- uint8_t iocb[64];
- } iocb;
+ uint8_t iocb[] __counted_by(size);
};
#include "qla_edif.h"
@@ -5101,7 +5099,6 @@ typedef struct scsi_qla_host {
struct list_head head;
spinlock_t lock;
} purex_list;
- struct purex_item default_item;
struct name_list_extended gnl;
/* Count of active session/fcport */
@@ -5130,6 +5127,11 @@ typedef struct scsi_qla_host {
#define DPORT_DIAG_IN_PROGRESS BIT_0
#define DPORT_DIAG_CHIP_RESET_IN_PROGRESS BIT_1
uint16_t dport_status;
+
+ /* Must be last --ends in a flexible-array member. */
+ TRAILING_OVERLAP(struct purex_item, default_item, iocb,
+ uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
+ );
} scsi_qla_host_t;
struct qla27xx_image_status {
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 8ff8781dae47..ccb044693dcb 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1078,17 +1078,17 @@ static struct purex_item *
qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
{
struct purex_item *item = NULL;
- uint8_t item_hdr_size = sizeof(*item);
if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
- item = kzalloc(item_hdr_size +
- (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
+ item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
} else {
if (atomic_inc_return(&vha->default_item.in_use) == 1) {
item = &vha->default_item;
goto initialize_purex_header;
} else {
- item = kzalloc(item_hdr_size, GFP_ATOMIC);
+ item = kzalloc(
+ struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
+ GFP_ATOMIC);
}
}
if (!item) {
@@ -1128,17 +1128,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
* @vha: SCSI driver HA context
* @pkt: ELS packet
*/
-static struct purex_item
-*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
+static struct purex_item *
+qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
{
struct purex_item *item;
- item = qla24xx_alloc_purex_item(vha,
- QLA_DEFAULT_PAYLOAD_SIZE);
+ item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
if (!item)
return item;
- memcpy(&item->iocb, pkt, sizeof(item->iocb));
+ memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
return item;
}
diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 8ee2e337c9e1..92488890bc04 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
ql_dbg(ql_dbg_unsol, vha, 0x2121,
"PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
- item->iocb.iocb[3], item->size, uctx->exchange_address,
+ item->iocb[3], item->size, uctx->exchange_address,
fcport->d_id.b24);
/* +48 0 1 2 3 4 5 6 7 8 9 A B C D E F
* ----- -----------------------------------------------
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index d4b484c0fd9d..32437bae1a30 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -6459,9 +6459,12 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
void
qla24xx_free_purex_item(struct purex_item *item)
{
- if (item == &item->vha->default_item)
- memset(&item->vha->default_item, 0, sizeof(struct purex_item));
- else
+ scsi_qla_host_t *base_vha = item->vha;
+
+ if (item == &base_vha->default_item) {
+ memset(&base_vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
+ memset(&base_vha->default_item, 0, sizeof(struct purex_item));
+ } else
kfree(item);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v10 11/11] scsi: qla2xxx: Fix 2 memcpy field-spanning write issue
2025-09-26 0:02 ` [PATCH v10 11/11] scsi: qla2xxx: Fix 2 memcpy field-spanning write issue John Meneghini
@ 2025-09-26 9:00 ` Gustavo A. R. Silva
2025-09-26 9:29 ` Hannes Reinecke
2025-09-30 9:24 ` John Meneghini
0 siblings, 2 replies; 21+ messages in thread
From: Gustavo A. R. Silva @ 2025-09-26 9:00 UTC (permalink / raw)
To: John Meneghini, hare, kbusch, martin.petersen, linux-nvme,
linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, kees,
linux-hardening, njavali, sagi
Hi,
Shouldn't this patch be removed from this series, since it's going to be
reverted anyways?
Thanks
-Gustavo
On 9/26/25 02:02, John Meneghini wrote:
> From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
>
> purex_item.iocb is defined as a 64-element u8 array, but 64 is the
> minimum size and it can be allocated larger. This makes it a standard
> empty flex array.
>
> This was motivated by field-spanning write warnings during FPIN testing.
>
> > kernel: memcpy: detected field-spanning write (size 60) of single
> > field "((uint8_t *)fpin_pkt + buffer_copy_offset)"
> > at drivers/scsi/qla2xxx/qla_isr.c:1221 (size 44)
>
> I removed the outer wrapper from the iocb flex array, so that it can be
> linked to `purex_item.size` with `__counted_by`.
>
> These changes remove the default minimum 64-byte allocation, requiring
> further changes.
>
> In `struct scsi_qla_host` the embedded `default_item` is now followed
> by `__default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE]` to reserve space
> that will be used as `default_item.iocb`. This is wrapped using the
> `TRAILING_OVERLAP()` macro helper, which effectively creates a union
> between flexible-array member `default_item.iocb` and
> `__default_item_iocb`.
>
> Since `struct pure_item` now contains a flexible-array member, the
> helper must be placed at the end of `struct scsi_qla_host` to prevent
> a `-Wflex-array-member-not-at-end` warning.
>
> `qla24xx_alloc_purex_item()` is adjusted to no longer expect the
> default minimum size to be part of `sizeof(struct purex_item)`,
> the entire flexible array size is added to the structure size for
> allocation.
>
> This also slightly changes the layout of the purex_item struct, as
> 2-bytes of padding are added between `size` and `iocb`. The resulting
> size is the same, but iocb is shifted 2-bytes (the original `purex_item`
> structure was padded at the end, after the 64-byte defined array size).
> I don't think this is a problem.
>
> In qla_os.c:qla24xx_process_purex_rdp()
>
> To avoid a null pointer dereference the vha->default_item should be set
> to 0 last if the item pointer passed to the function matches. Also use
> a local variable to avoid multiple de-referencing of the item.
>
> Tested-by: Bryan Gurney <bgurney@redhat.com>
> Co-developed-by: Chris Leech <cleech@redhat.com>
> Signed-off-by: Chris Leech <cleech@redhat.com>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> drivers/scsi/qla2xxx/qla_def.h | 10 ++++++----
> drivers/scsi/qla2xxx/qla_isr.c | 17 ++++++++---------
> drivers/scsi/qla2xxx/qla_nvme.c | 2 +-
> drivers/scsi/qla2xxx/qla_os.c | 9 ++++++---
> 4 files changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
> index cb95b7b12051..604e66bead1e 100644
> --- a/drivers/scsi/qla2xxx/qla_def.h
> +++ b/drivers/scsi/qla2xxx/qla_def.h
> @@ -4890,9 +4890,7 @@ struct purex_item {
> struct purex_item *pkt);
> atomic_t in_use;
> uint16_t size;
> - struct {
> - uint8_t iocb[64];
> - } iocb;
> + uint8_t iocb[] __counted_by(size);
> };
>
> #include "qla_edif.h"
> @@ -5101,7 +5099,6 @@ typedef struct scsi_qla_host {
> struct list_head head;
> spinlock_t lock;
> } purex_list;
> - struct purex_item default_item;
>
> struct name_list_extended gnl;
> /* Count of active session/fcport */
> @@ -5130,6 +5127,11 @@ typedef struct scsi_qla_host {
> #define DPORT_DIAG_IN_PROGRESS BIT_0
> #define DPORT_DIAG_CHIP_RESET_IN_PROGRESS BIT_1
> uint16_t dport_status;
> +
> + /* Must be last --ends in a flexible-array member. */
> + TRAILING_OVERLAP(struct purex_item, default_item, iocb,
> + uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
> + );
> } scsi_qla_host_t;
>
> struct qla27xx_image_status {
> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
> index 8ff8781dae47..ccb044693dcb 100644
> --- a/drivers/scsi/qla2xxx/qla_isr.c
> +++ b/drivers/scsi/qla2xxx/qla_isr.c
> @@ -1078,17 +1078,17 @@ static struct purex_item *
> qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
> {
> struct purex_item *item = NULL;
> - uint8_t item_hdr_size = sizeof(*item);
>
> if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
> - item = kzalloc(item_hdr_size +
> - (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
> + item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
> } else {
> if (atomic_inc_return(&vha->default_item.in_use) == 1) {
> item = &vha->default_item;
> goto initialize_purex_header;
> } else {
> - item = kzalloc(item_hdr_size, GFP_ATOMIC);
> + item = kzalloc(
> + struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
> + GFP_ATOMIC);
> }
> }
> if (!item) {
> @@ -1128,17 +1128,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
> * @vha: SCSI driver HA context
> * @pkt: ELS packet
> */
> -static struct purex_item
> -*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
> +static struct purex_item *
> +qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
> {
> struct purex_item *item;
>
> - item = qla24xx_alloc_purex_item(vha,
> - QLA_DEFAULT_PAYLOAD_SIZE);
> + item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
> if (!item)
> return item;
>
> - memcpy(&item->iocb, pkt, sizeof(item->iocb));
> + memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
> return item;
> }
>
> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
> index 8ee2e337c9e1..92488890bc04 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
>
> ql_dbg(ql_dbg_unsol, vha, 0x2121,
> "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
> - item->iocb.iocb[3], item->size, uctx->exchange_address,
> + item->iocb[3], item->size, uctx->exchange_address,
> fcport->d_id.b24);
> /* +48 0 1 2 3 4 5 6 7 8 9 A B C D E F
> * ----- -----------------------------------------------
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index d4b484c0fd9d..32437bae1a30 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -6459,9 +6459,12 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
> void
> qla24xx_free_purex_item(struct purex_item *item)
> {
> - if (item == &item->vha->default_item)
> - memset(&item->vha->default_item, 0, sizeof(struct purex_item));
> - else
> + scsi_qla_host_t *base_vha = item->vha;
> +
> + if (item == &base_vha->default_item) {
> + memset(&base_vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
> + memset(&base_vha->default_item, 0, sizeof(struct purex_item));
> + } else
> kfree(item);
> }
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v10 11/11] scsi: qla2xxx: Fix 2 memcpy field-spanning write issue
2025-09-26 9:00 ` Gustavo A. R. Silva
@ 2025-09-26 9:29 ` Hannes Reinecke
2025-09-30 9:38 ` John Meneghini
2025-09-30 9:24 ` John Meneghini
1 sibling, 1 reply; 21+ messages in thread
From: Hannes Reinecke @ 2025-09-26 9:29 UTC (permalink / raw)
To: Gustavo A. R. Silva, John Meneghini, kbusch, martin.petersen,
linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, kees,
linux-hardening, njavali, sagi
On 9/26/25 11:00, Gustavo A. R. Silva wrote:
> Hi,
>
> Shouldn't this patch be removed from this series, since it's going to be
> reverted anyways?
>
yes. To my understanding the FPIN patch series has been queued in
scsi-queue anyway, so it would be better to just send an incremental
patch on top of that.
Especially as Martin has already indicated that he will _not_ rebase
his tree.
Best to just send this patch as a stand-alone patch, and then rebase
any not-yet-upstreamed patchsets on top of that.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v10 11/11] scsi: qla2xxx: Fix 2 memcpy field-spanning write issue
2025-09-26 9:29 ` Hannes Reinecke
@ 2025-09-30 9:38 ` John Meneghini
0 siblings, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-30 9:38 UTC (permalink / raw)
To: Hannes Reinecke, Gustavo A. R. Silva, kbusch, martin.petersen,
linux-nvme, linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, kees,
linux-hardening, njavali, sagi
On 9/26/25 5:29 AM, Hannes Reinecke wrote:
> On 9/26/25 11:00, Gustavo A. R. Silva wrote:
>> Hi,
>>
>> Shouldn't this patch be removed from this series, since it's going to be
>> reverted anyways?
>>
> yes. To my understanding the FPIN patch series has been queued in
> scsi-queue anyway, so it would be better to just send an incremental
> patch on top of that.
> Especially as Martin has already indicated that he will _not_ rebase
> his tree.
This V10 patch series is based on nvme-v18.
I don't see the revert in scsi/6.18/scsi-queue yet
> Best to just send this patch as a stand-alone patch, and then rebase
> any not-yet-upstreamed patchsets on top of that.
I'd prefer to merge this patch with the FPIN-LI patch series. There is to good way to test this field-spanning write fix w/out the FPIN_LI patches,
and the last time we cherry-picked and merged this fix it only led to confusion.
I think we should keep these together. I will add Gustavo's new fix to my V11 patch series.
/John
> Cheers,
>
> Hannes
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v10 11/11] scsi: qla2xxx: Fix 2 memcpy field-spanning write issue
2025-09-26 9:00 ` Gustavo A. R. Silva
2025-09-26 9:29 ` Hannes Reinecke
@ 2025-09-30 9:24 ` John Meneghini
1 sibling, 0 replies; 21+ messages in thread
From: John Meneghini @ 2025-09-30 9:24 UTC (permalink / raw)
To: Gustavo A. R. Silva, hare, kbusch, martin.petersen, linux-nvme,
linux-scsi
Cc: bgurney, axboe, emilne, gustavoars, hch, james.smart, kees,
linux-hardening, njavali, sagi
Hi Gustavo.
Sorry for my late reply.
Yes, I will remove this patch and replaces it with your proposed patch[1] from the V9 patch email thread.
I see that Bryan has already tested your new patch and it works. I agree the new patch is a much simpler/better version.
[1] https://lore.kernel.org/linux-nvme/97526d45-ec7d-48a0-bdc6-659f75839f53@embeddedor.com/#t
I will add this to my V11 patch series.
/John
On 9/26/25 5:00 AM, Gustavo A. R. Silva wrote:
> Hi,
>
> Shouldn't this patch be removed from this series, since it's going to be
> reverted anyways?
>
> Thanks
> -Gustavo
>
> On 9/26/25 02:02, John Meneghini wrote:
>> From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
>>
>> purex_item.iocb is defined as a 64-element u8 array, but 64 is the
>> minimum size and it can be allocated larger. This makes it a standard
>> empty flex array.
>>
>> This was motivated by field-spanning write warnings during FPIN testing.
>>
>> > kernel: memcpy: detected field-spanning write (size 60) of single
>> > field "((uint8_t *)fpin_pkt + buffer_copy_offset)"
>> > at drivers/scsi/qla2xxx/qla_isr.c:1221 (size 44)
>>
>> I removed the outer wrapper from the iocb flex array, so that it can be
>> linked to `purex_item.size` with `__counted_by`.
>>
>> These changes remove the default minimum 64-byte allocation, requiring
>> further changes.
>>
>> In `struct scsi_qla_host` the embedded `default_item` is now followed
>> by `__default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE]` to reserve space
>> that will be used as `default_item.iocb`. This is wrapped using the
>> `TRAILING_OVERLAP()` macro helper, which effectively creates a union
>> between flexible-array member `default_item.iocb` and
>> `__default_item_iocb`.
>>
>> Since `struct pure_item` now contains a flexible-array member, the
>> helper must be placed at the end of `struct scsi_qla_host` to prevent
>> a `-Wflex-array-member-not-at-end` warning.
>>
>> `qla24xx_alloc_purex_item()` is adjusted to no longer expect the
>> default minimum size to be part of `sizeof(struct purex_item)`,
>> the entire flexible array size is added to the structure size for
>> allocation.
>>
>> This also slightly changes the layout of the purex_item struct, as
>> 2-bytes of padding are added between `size` and `iocb`. The resulting
>> size is the same, but iocb is shifted 2-bytes (the original `purex_item`
>> structure was padded at the end, after the 64-byte defined array size).
>> I don't think this is a problem.
>>
>> In qla_os.c:qla24xx_process_purex_rdp()
>>
>> To avoid a null pointer dereference the vha->default_item should be set
>> to 0 last if the item pointer passed to the function matches. Also use
>> a local variable to avoid multiple de-referencing of the item.
>>
>> Tested-by: Bryan Gurney <bgurney@redhat.com>
>> Co-developed-by: Chris Leech <cleech@redhat.com>
>> Signed-off-by: Chris Leech <cleech@redhat.com>
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>> drivers/scsi/qla2xxx/qla_def.h | 10 ++++++----
>> drivers/scsi/qla2xxx/qla_isr.c | 17 ++++++++---------
>> drivers/scsi/qla2xxx/qla_nvme.c | 2 +-
>> drivers/scsi/qla2xxx/qla_os.c | 9 ++++++---
>> 4 files changed, 21 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h
>> index cb95b7b12051..604e66bead1e 100644
>> --- a/drivers/scsi/qla2xxx/qla_def.h
>> +++ b/drivers/scsi/qla2xxx/qla_def.h
>> @@ -4890,9 +4890,7 @@ struct purex_item {
>> struct purex_item *pkt);
>> atomic_t in_use;
>> uint16_t size;
>> - struct {
>> - uint8_t iocb[64];
>> - } iocb;
>> + uint8_t iocb[] __counted_by(size);
>> };
>> #include "qla_edif.h"
>> @@ -5101,7 +5099,6 @@ typedef struct scsi_qla_host {
>> struct list_head head;
>> spinlock_t lock;
>> } purex_list;
>> - struct purex_item default_item;
>> struct name_list_extended gnl;
>> /* Count of active session/fcport */
>> @@ -5130,6 +5127,11 @@ typedef struct scsi_qla_host {
>> #define DPORT_DIAG_IN_PROGRESS BIT_0
>> #define DPORT_DIAG_CHIP_RESET_IN_PROGRESS BIT_1
>> uint16_t dport_status;
>> +
>> + /* Must be last --ends in a flexible-array member. */
>> + TRAILING_OVERLAP(struct purex_item, default_item, iocb,
>> + uint8_t __default_item_iocb[QLA_DEFAULT_PAYLOAD_SIZE];
>> + );
>> } scsi_qla_host_t;
>> struct qla27xx_image_status {
>> diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
>> index 8ff8781dae47..ccb044693dcb 100644
>> --- a/drivers/scsi/qla2xxx/qla_isr.c
>> +++ b/drivers/scsi/qla2xxx/qla_isr.c
>> @@ -1078,17 +1078,17 @@ static struct purex_item *
>> qla24xx_alloc_purex_item(scsi_qla_host_t *vha, uint16_t size)
>> {
>> struct purex_item *item = NULL;
>> - uint8_t item_hdr_size = sizeof(*item);
>> if (size > QLA_DEFAULT_PAYLOAD_SIZE) {
>> - item = kzalloc(item_hdr_size +
>> - (size - QLA_DEFAULT_PAYLOAD_SIZE), GFP_ATOMIC);
>> + item = kzalloc(struct_size(item, iocb, size), GFP_ATOMIC);
>> } else {
>> if (atomic_inc_return(&vha->default_item.in_use) == 1) {
>> item = &vha->default_item;
>> goto initialize_purex_header;
>> } else {
>> - item = kzalloc(item_hdr_size, GFP_ATOMIC);
>> + item = kzalloc(
>> + struct_size(item, iocb, QLA_DEFAULT_PAYLOAD_SIZE),
>> + GFP_ATOMIC);
>> }
>> }
>> if (!item) {
>> @@ -1128,17 +1128,16 @@ qla24xx_queue_purex_item(scsi_qla_host_t *vha, struct purex_item *pkt,
>> * @vha: SCSI driver HA context
>> * @pkt: ELS packet
>> */
>> -static struct purex_item
>> -*qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
>> +static struct purex_item *
>> +qla24xx_copy_std_pkt(struct scsi_qla_host *vha, void *pkt)
>> {
>> struct purex_item *item;
>> - item = qla24xx_alloc_purex_item(vha,
>> - QLA_DEFAULT_PAYLOAD_SIZE);
>> + item = qla24xx_alloc_purex_item(vha, QLA_DEFAULT_PAYLOAD_SIZE);
>> if (!item)
>> return item;
>> - memcpy(&item->iocb, pkt, sizeof(item->iocb));
>> + memcpy(&item->iocb, pkt, QLA_DEFAULT_PAYLOAD_SIZE);
>> return item;
>> }
>> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
>> index 8ee2e337c9e1..92488890bc04 100644
>> --- a/drivers/scsi/qla2xxx/qla_nvme.c
>> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
>> @@ -1308,7 +1308,7 @@ void qla2xxx_process_purls_iocb(void **pkt, struct rsp_que **rsp)
>> ql_dbg(ql_dbg_unsol, vha, 0x2121,
>> "PURLS OP[%01x] size %d xchg addr 0x%x portid %06x\n",
>> - item->iocb.iocb[3], item->size, uctx->exchange_address,
>> + item->iocb[3], item->size, uctx->exchange_address,
>> fcport->d_id.b24);
>> /* +48 0 1 2 3 4 5 6 7 8 9 A B C D E F
>> * ----- -----------------------------------------------
>> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
>> index d4b484c0fd9d..32437bae1a30 100644
>> --- a/drivers/scsi/qla2xxx/qla_os.c
>> +++ b/drivers/scsi/qla2xxx/qla_os.c
>> @@ -6459,9 +6459,12 @@ void qla24xx_process_purex_rdp(struct scsi_qla_host *vha,
>> void
>> qla24xx_free_purex_item(struct purex_item *item)
>> {
>> - if (item == &item->vha->default_item)
>> - memset(&item->vha->default_item, 0, sizeof(struct purex_item));
>> - else
>> + scsi_qla_host_t *base_vha = item->vha;
>> +
>> + if (item == &base_vha->default_item) {
>> + memset(&base_vha->__default_item_iocb, 0, QLA_DEFAULT_PAYLOAD_SIZE);
>> + memset(&base_vha->default_item, 0, sizeof(struct purex_item));
>> + } else
>> kfree(item);
>> }
>
^ permalink raw reply [flat|nested] 21+ messages in thread