> On Mar 1, 2022, at 9:55 AM, James Smart <jsmart2021@xxxxxxxxx> wrote:
>
> In the original FPIN commit, stats were incremented by the event_count.
> Event_count is the minimum # of events that must occur before an FPIN is
> sent. Thus, its not the actual number of events, and could be
> significantly off (too low) as it doesn't reflect anything not reported.
> Rather than attempt to count events, have the statistic count how many
> FPINS cross the threshold and were reported.
>
> Fixes: 3dcfe0de5a97 ("scsi: fc: Parse FPIN packets and update statistics")
> Cc: <stable@xxxxxxxxxxxxxxx> # v5.11+
> Signed-off-by: James Smart <jsmart2021@xxxxxxxxx>
> Cc: Shyam Sundar <ssundar@xxxxxxxxxxx>
> Cc: Nilesh Javali <njavali@xxxxxxxxxxx>
>
> ---
> This issue was originally reported in this thread, with no comments.
> https://lore.kernel.org/linux-scsi/b472606d-e67c-66f1-06d1-ecc5fbb2071a@xxxxxxxxxxxx/
> ---
> drivers/scsi/scsi_transport_fc.c | 39 +++++++++++++-------------------
> 1 file changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 60e406bcf42a..a2524106206d 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -34,7 +34,7 @@ static int fc_bsg_hostadd(struct Scsi_Host *, struct fc_host_attrs *);
> static int fc_bsg_rportadd(struct Scsi_Host *, struct fc_rport *);
> static void fc_bsg_remove(struct request_queue *);
> static void fc_bsg_goose_queue(struct fc_rport *);
> -static void fc_li_stats_update(struct fc_fn_li_desc *li_desc,
> +static void fc_li_stats_update(u16 event_type,
> struct fc_fpin_stats *stats);
> static void fc_delivery_stats_update(u32 reason_code,
> struct fc_fpin_stats *stats);
> @@ -670,42 +670,34 @@ fc_find_rport_by_wwpn(struct Scsi_Host *shost, u64 wwpn)
> EXPORT_SYMBOL(fc_find_rport_by_wwpn);
>
> static void
> -fc_li_stats_update(struct fc_fn_li_desc *li_desc,
> +fc_li_stats_update(u16 event_type,
> struct fc_fpin_stats *stats)
> {
> - stats->li += be32_to_cpu(li_desc->event_count);
> - switch (be16_to_cpu(li_desc->event_type)) {
> + stats->li++;
> + switch (event_type) {
> case FPIN_LI_UNKNOWN:
> - stats->li_failure_unknown +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_failure_unknown++;
> break;
> case FPIN_LI_LINK_FAILURE:
> - stats->li_link_failure_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_link_failure_count++;
> break;
> case FPIN_LI_LOSS_OF_SYNC:
> - stats->li_loss_of_sync_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_loss_of_sync_count++;
> break;
> case FPIN_LI_LOSS_OF_SIG:
> - stats->li_loss_of_signals_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_loss_of_signals_count++;
> break;
> case FPIN_LI_PRIM_SEQ_ERR:
> - stats->li_prim_seq_err_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_prim_seq_err_count++;
> break;
> case FPIN_LI_INVALID_TX_WD:
> - stats->li_invalid_tx_word_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_invalid_tx_word_count++;
> break;
> case FPIN_LI_INVALID_CRC:
> - stats->li_invalid_crc_count +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_invalid_crc_count++;
> break;
> case FPIN_LI_DEVICE_SPEC:
> - stats->li_device_specific +=
> - be32_to_cpu(li_desc->event_count);
> + stats->li_device_specific++;
> break;
> }
> }
> @@ -767,6 +759,7 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
> 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;
>
> rport = fc_find_rport_by_wwpn(shost,
> @@ -775,7 +768,7 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
> (rport->roles & FC_PORT_ROLE_FCP_TARGET ||
> rport->roles & FC_PORT_ROLE_NVME_TARGET)) {
> attach_rport = rport;
> - fc_li_stats_update(li_desc, &attach_rport->fpin_stats);
> + fc_li_stats_update(event_type, &attach_rport->fpin_stats);
> }
>
> if (be32_to_cpu(li_desc->pname_count) > 0) {
> @@ -789,14 +782,14 @@ fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
> rport->roles & FC_PORT_ROLE_NVME_TARGET)) {
> if (rport == attach_rport)
> continue;
> - fc_li_stats_update(li_desc,
> + fc_li_stats_update(event_type,
> &rport->fpin_stats);
> }
> }
> }
>
> if (fc_host->port_name == be64_to_cpu(li_desc->attached_wwpn))
> - fc_li_stats_update(li_desc, &fc_host->fpin_stats);
> + fc_li_stats_update(event_type, &fc_host->fpin_stats);
> }
>
> /*
> --
> 2.26.2
>
Looks Good.
Reviewed-by: Himanshu Madhani <himanshu.madhani@xxxxxxxxxx>
--
Himanshu Madhani Oracle Linux Engineering
