Fpin v11g - #14
Draft
johnmeneghini wants to merge 16 commits into
Draft
Conversation
This reverts commit 19bc5f2. So we have been seeing failures in our internal qla2xxx FPIN testing for a while. These failures are due to this commit and has been reported internally at Red Hat. Recently Sustaining Engineering asked us to put in a patch to resolve this issue. The fix is to rever the patch: commit 19bc5f2 Author: Jiasheng Jiang <jiashengjiangcool@gmail.com> Date: Tue Jan 6 20:53:44 2026 +0000 scsi: qla2xxx: Sanitize payload size to prevent member overflow Unfortunately this fix does not work. Moveover, truncating the received FPIN data seemed like a bad idea in general. There was a previous attempt to fix this, but it did not work right either: commit 6f4b102 Author: Gustavo A. R. Silva <gustavoars@kernel.org> Date: Wed Aug 13 16:07:44 2025 -0400 scsi: qla2xxx: Fix memcpy() field-spanning write issue So John reverted it: commit 285654d Author: John Meneghini <jmeneghi@redhat.com> Date: Thu Sep 25 09:07:29 2025 -0400 Revert "scsi: qla2xxx: Fix memcpy() field-spanning write issue" Signed-off-by: John Meneghini <jmeneghi@redhat.com> Reviewed-by: Ewan D. Milne <emilne@redhat.com> Reviewed-by: David Jeffery <djeffery@redhat.com>
Introduce 'union fc_tlv_desc' to have a common structure for all FC ELS TLV structures and avoid type casts. Fixes: 44b6169 ("scsi: fc: Avoid -Wflex-array-member-not-at-end warnings") Commit 44b6169 ("scsi: fc: Avoid -Wflex-array-member-not-at-end warnings") conflicts with this change. To solve the problem I moved the fc_els_rdf_hdr and fc_els_rdf_resp_hdr definitions out of fc_els.h and into lpfc_hw4.h. Cc: Gustavo A. R. Silva <gustavoars@kernel.org> Co-developed-by: John Meneghini <jmeneghi@redhat.com> Signed-off-by: John Meneghini <jmeneghi@redhat.com> Signed-off-by: Hannes Reinecke <hare@kernel.org> Tested-by: 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>
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 no other non-marginal paths are available. 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. Signed-off-by: Jesse Taube <jtaubepe@redhat.com> --- This is a distinct change from the previous commit which treated marginal paths as non-optimized but still usable. This changes the priority of marginal paths to be lower than non-optimized paths.
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 and select the best marginal path. Signed-off-by: Jesse Taube <jtaubepe@redhat.com> Signed-off-by: John Meneghini <jmeneghi@redhat.com>
Exclude marginal paths from round-robin io policy. In the case where all paths are marginal and no optimized or non-optimized path is found, we fall back and perform round-robin on the marginal paths. Signed-off-by: Jesse Taube <jtaubepe@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>
Add fc_fpin_set_marginal function to evaluate the FPIN LI TLV information and set the 'marginal' path status for all affected nvme rports. Then call set_rport_marginal() callback added to fc_function_template. The implementation of set_rport_marginal() is almost identical to set_rport_dev_loss_tmo implementation. 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> --- why do we check rport->roles & FC_PORT_ROLE_NVME_TARGET but not in the sysfs commit
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>
Add nvme_fc_set_remoteport_fpin() 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>
Implement set_rport_marginal callback using nvme_fc_set_remoteport_fpin() 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>
Implement set_rport_marginal callback using nvme_fc_set_remoteport_fpin() 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>
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>
This is useful for comparing the fpin packets on lpfc vs qla2xxx to see if there are differences that could explain why the qla2xxx driver is not working. It was found that the qla2xxx driver is providing a truncated version of the fpin packet to fc_host_fpin_rcv. Signed-off-by: Jesse Taube <jtaubepe@redhat.com>
johnmeneghini
marked this pull request as draft
June 30, 2026 19:12
Signed-off-by: John Meneghini <jmeneghi@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Latest FPIN LI changes for nvme-linux native multipath