diff mbox series

[v2,3/5] scsi: fc: Parse FPIN packets and update statistics

Message ID 20201006061615.28674-4-njavali@marvell.com
State New
Headers show
Series [v2,1/5] scsi: fc: Update formal FPIN descriptor definitions | expand

Commit Message

Nilesh Javali Oct. 6, 2020, 6:16 a.m. UTC
From: Shyam Sundar <ssundar@marvell.com>

Parse the incoming FPIN packets and update the host and rport FPIN
statistics based on the FPINs.

Signed-off-by: Shyam Sundar <ssundar@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/scsi_transport_fc.c | 322 +++++++++++++++++++++++++++++++
 include/scsi/scsi_transport_fc.h |   1 +
 2 files changed, 323 insertions(+)

Comments

James Smart Oct. 12, 2020, 6:37 p.m. UTC | #1
On 10/5/2020 11:16 PM, Nilesh Javali wrote:
> From: Shyam Sundar <ssundar@marvell.com>

>

> Parse the incoming FPIN packets and update the host and rport FPIN

> statistics based on the FPINs.

>

> Signed-off-by: Shyam Sundar <ssundar@marvell.com>

> Signed-off-by: Nilesh Javali <njavali@marvell.com>

> ---

> ...

> +/*

> + * fc_fpin_li_stats_update - routine to update Link Integrity

> + * event statistics.

> + * @shost:		host the FPIN was received on

> + * @tlv:		pointer to link integrity descriptor

> + *

> + */

> +static void

> +fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)

> +{

> +	u8 i;

> +	struct fc_rport *rport = NULL;

> +	struct fc_rport *det_rport = NULL, *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;

> +	u64 wwpn;

> +

> +	rport = fc_find_rport_by_wwpn(shost,

> +				      be64_to_cpu(li_desc->detecting_wwpn));

> +	if (rport &&

> +	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||

> +	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {

> +		det_rport = rport;

> +		fc_li_stats_update(li_desc, &det_rport->fpin_stats);

> +	}


I thought we agreed to not include the detecting port in stat updates.

> +

> +	rport = fc_find_rport_by_wwpn(shost,

> +				      be64_to_cpu(li_desc->attached_wwpn));

> +	if (rport &&

> +	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||

> +	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {


nits:  indent the last line by 1 more space to align elements; 
superfluous parens on role checks (i.e. the inner conditions don't need 
to be parenthesized as order of precedence works fine).


same comments apply to the other parts.

-- james
Shyam Sundar Oct. 12, 2020, 7:03 p.m. UTC | #2
James,
     My apologies, I mis-read your comment/response below. While reading "let's not change", I interpreted it to "let's leave the patch as it is" 😊
     I'll fix that, and take care of the indentation in v3.

Regards
Shyam

On 9/28/2020 1:07 PM, Shyam Sundar wrote:
> I am open to removing the accounting against the "detecting" port for now, given that currently, there are no known implementations where the N_Port initiates the FPIN ELS.

> Let me know what you think.


Ok - let's not change counters on the "detecting" port.

On 10/12/20, 11:37 AM, "James Smart" <james.smart@broadcom.com> wrote:



    On 10/5/2020 11:16 PM, Nilesh Javali wrote:
    > From: Shyam Sundar <ssundar@marvell.com>

    >

    > Parse the incoming FPIN packets and update the host and rport FPIN

    > statistics based on the FPINs.

    >

    > Signed-off-by: Shyam Sundar <ssundar@marvell.com>

    > Signed-off-by: Nilesh Javali <njavali@marvell.com>

    > ---

    > ...

    > +/*

    > + * fc_fpin_li_stats_update - routine to update Link Integrity

    > + * event statistics.

    > + * @shost:		host the FPIN was received on

    > + * @tlv:		pointer to link integrity descriptor

    > + *

    > + */

    > +static void

    > +fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)

    > +{

    > +	u8 i;

    > +	struct fc_rport *rport = NULL;

    > +	struct fc_rport *det_rport = NULL, *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;

    > +	u64 wwpn;

    > +

    > +	rport = fc_find_rport_by_wwpn(shost,

    > +				      be64_to_cpu(li_desc->detecting_wwpn));

    > +	if (rport &&

    > +	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||

    > +	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {

    > +		det_rport = rport;

    > +		fc_li_stats_update(li_desc, &det_rport->fpin_stats);

    > +	}


    I thought we agreed to not include the detecting port in stat updates.

    > +

    > +	rport = fc_find_rport_by_wwpn(shost,

    > +				      be64_to_cpu(li_desc->attached_wwpn));

    > +	if (rport &&

    > +	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||

    > +	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {


    nits:  indent the last line by 1 more space to align elements; 
    superfluous parens on role checks (i.e. the inner conditions don't need 
    to be parenthesized as order of precedence works fine).


    same comments apply to the other parts.

    -- james
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 501e165ae6f1..3db7eb674cda 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -34,6 +34,11 @@  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,
+			       struct fc_fpin_stats *stats);
+static void fc_delivery_stats_update(u32 reason_code,
+				     struct fc_fpin_stats *stats);
+static void fc_cn_stats_update(u16 event_type, struct fc_fpin_stats *stats);
 
 /*
  * Module Parameters
@@ -630,6 +635,291 @@  fc_host_post_vendor_event(struct Scsi_Host *shost, u32 event_number,
 }
 EXPORT_SYMBOL(fc_host_post_vendor_event);
 
+/**
+ * fc_find_rport_by_wwpn - find the fc_rport pointer for a given wwpn
+ * @shost:		host the fc_rport is associated with
+ * @wwpn:		wwpn of the fc_rport device
+ *
+ * Notes:
+ *	This routine assumes no locks are held on entry.
+ */
+struct fc_rport *
+fc_find_rport_by_wwpn(struct Scsi_Host *shost, u64 wwpn)
+{
+	struct fc_rport *rport;
+	unsigned long flags;
+
+	spin_lock_irqsave(shost->host_lock, flags);
+
+	list_for_each_entry(rport, &fc_host_rports(shost), peers) {
+		if (rport->port_state != FC_PORTSTATE_ONLINE)
+			continue;
+
+		if (rport->port_name == wwpn) {
+			spin_unlock_irqrestore(shost->host_lock, flags);
+			return rport;
+		}
+	}
+
+	spin_unlock_irqrestore(shost->host_lock, flags);
+	return NULL;
+}
+EXPORT_SYMBOL(fc_find_rport_by_wwpn);
+
+static void
+fc_li_stats_update(struct fc_fn_li_desc *li_desc,
+		   struct fc_fpin_stats *stats)
+{
+	stats->li += be32_to_cpu(li_desc->event_count);
+	switch (be16_to_cpu(li_desc->event_type)) {
+	case FPIN_LI_UNKNOWN:
+		stats->li_failure_unknown +=
+		    be32_to_cpu(li_desc->event_count);
+		break;
+	case FPIN_LI_LINK_FAILURE:
+		stats->li_link_failure_count +=
+		    be32_to_cpu(li_desc->event_count);
+		break;
+	case FPIN_LI_LOSS_OF_SYNC:
+		stats->li_loss_of_sync_count +=
+		    be32_to_cpu(li_desc->event_count);
+		break;
+	case FPIN_LI_LOSS_OF_SIG:
+		stats->li_loss_of_signals_count +=
+		    be32_to_cpu(li_desc->event_count);
+		break;
+	case FPIN_LI_PRIM_SEQ_ERR:
+		stats->li_prim_seq_err_count +=
+		    be32_to_cpu(li_desc->event_count);
+		break;
+	case FPIN_LI_INVALID_TX_WD:
+		stats->li_invalid_tx_word_count +=
+		    be32_to_cpu(li_desc->event_count);
+		break;
+	case FPIN_LI_INVALID_CRC:
+		stats->li_invalid_crc_count +=
+		    be32_to_cpu(li_desc->event_count);
+		break;
+	case FPIN_LI_DEVICE_SPEC:
+		stats->li_device_specific +=
+		    be32_to_cpu(li_desc->event_count);
+		break;
+	}
+}
+
+static void
+fc_delivery_stats_update(u32 reason_code, struct fc_fpin_stats *stats)
+{
+	stats->dn++;
+	switch (reason_code) {
+	case FPIN_DELI_UNKNOWN:
+		stats->dn_unknown++;
+		break;
+	case FPIN_DELI_TIMEOUT:
+		stats->dn_timeout++;
+		break;
+	case FPIN_DELI_UNABLE_TO_ROUTE:
+		stats->dn_unable_to_route++;
+		break;
+	case FPIN_DELI_DEVICE_SPEC:
+		stats->dn_device_specific++;
+		break;
+	}
+}
+
+static void
+fc_cn_stats_update(u16 event_type, struct fc_fpin_stats *stats)
+{
+	stats->cn++;
+	switch (event_type) {
+	case FPIN_CONGN_CLEAR:
+		stats->cn_clear++;
+		break;
+	case FPIN_CONGN_LOST_CREDIT:
+		stats->cn_lost_credit++;
+		break;
+	case FPIN_CONGN_CREDIT_STALL:
+		stats->cn_credit_stall++;
+		break;
+	case FPIN_CONGN_OVERSUBSCRIPTION:
+		stats->cn_oversubscription++;
+		break;
+	case FPIN_CONGN_DEVICE_SPEC:
+		stats->cn_device_specific++;
+	}
+}
+
+/*
+ * fc_fpin_li_stats_update - routine to update Link Integrity
+ * event statistics.
+ * @shost:		host the FPIN was received on
+ * @tlv:		pointer to link integrity descriptor
+ *
+ */
+static void
+fc_fpin_li_stats_update(struct Scsi_Host *shost, struct fc_tlv_desc *tlv)
+{
+	u8 i;
+	struct fc_rport *rport = NULL;
+	struct fc_rport *det_rport = NULL, *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;
+	u64 wwpn;
+
+	rport = fc_find_rport_by_wwpn(shost,
+				      be64_to_cpu(li_desc->detecting_wwpn));
+	if (rport &&
+	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||
+	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {
+		det_rport = rport;
+		fc_li_stats_update(li_desc, &det_rport->fpin_stats);
+	}
+
+	rport = fc_find_rport_by_wwpn(shost,
+				      be64_to_cpu(li_desc->attached_wwpn));
+	if (rport &&
+	    ((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);
+	}
+
+	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]);
+			rport = fc_find_rport_by_wwpn(shost, wwpn);
+			if (rport &&
+			    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||
+			    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {
+				if (rport == det_rport ||
+				    rport == attach_rport)
+					continue;
+				fc_li_stats_update(li_desc,
+						   &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_fpin_delivery_stats_update - routine to update Delivery Notification
+ * event statistics.
+ * @shost:		host the FPIN was received on
+ * @tlv:		pointer to delivery descriptor
+ *
+ */
+static void
+fc_fpin_delivery_stats_update(struct Scsi_Host *shost,
+			      struct fc_tlv_desc *tlv)
+{
+	struct fc_rport *rport = NULL;
+	struct fc_rport *det_rport = NULL, *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,
+				      be64_to_cpu(dn_desc->detecting_wwpn));
+	if (rport &&
+	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||
+	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {
+		det_rport = rport;
+		fc_delivery_stats_update(reason_code, &det_rport->fpin_stats);
+	}
+
+	rport = fc_find_rport_by_wwpn(shost,
+				      be64_to_cpu(dn_desc->attached_wwpn));
+	if (rport &&
+	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||
+	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {
+		attach_rport = rport;
+		fc_delivery_stats_update(reason_code,
+					 &attach_rport->fpin_stats);
+	}
+
+	if (fc_host->port_name == be64_to_cpu(dn_desc->attached_wwpn))
+		fc_delivery_stats_update(reason_code, &fc_host->fpin_stats);
+}
+
+/*
+ * fc_fpin_peer_congn_stats_update - routine to update Peer Congestion
+ * event statistics.
+ * @shost:		host the FPIN was received on
+ * @tlv:		pointer to peer congestion descriptor
+ *
+ */
+static void
+fc_fpin_peer_congn_stats_update(struct Scsi_Host *shost,
+				struct fc_tlv_desc *tlv)
+{
+	u8 i;
+	struct fc_rport *rport = NULL;
+	struct fc_rport *det_rport = NULL, *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;
+
+	rport = fc_find_rport_by_wwpn(shost,
+				      be64_to_cpu(pc_desc->detecting_wwpn));
+	if (rport &&
+	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||
+	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {
+		det_rport = rport;
+		fc_cn_stats_update(event_type, &det_rport->fpin_stats);
+	}
+
+	rport = fc_find_rport_by_wwpn(shost,
+				      be64_to_cpu(pc_desc->attached_wwpn));
+	if (rport &&
+	    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||
+	    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {
+		attach_rport = rport;
+		fc_cn_stats_update(event_type, &attach_rport->fpin_stats);
+	}
+
+	if (be32_to_cpu(pc_desc->pname_count) > 0) {
+		for (i = 0;
+		    i < be32_to_cpu(pc_desc->pname_count);
+		    i++) {
+			wwpn = be64_to_cpu(pc_desc->pname_list[i]);
+			rport = fc_find_rport_by_wwpn(shost, wwpn);
+			if (rport &&
+			    ((rport->roles & FC_PORT_ROLE_FCP_TARGET) ||
+			    (rport->roles & FC_PORT_ROLE_NVME_TARGET))) {
+				if (rport == det_rport ||
+				    rport == attach_rport)
+					continue;
+				fc_cn_stats_update(event_type,
+						   &rport->fpin_stats);
+			}
+		}
+	}
+}
+
+/*
+ * fc_fpin_congn_stats_update - routine to update Congestion
+ * event statistics.
+ * @shost:		host the FPIN was received on
+ * @tlv:		pointer to congestion descriptor
+ *
+ */
+static void
+fc_fpin_congn_stats_update(struct Scsi_Host *shost,
+			   struct fc_tlv_desc *tlv)
+{
+	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);
+}
+
 /**
  * fc_host_rcv_fpin - routine to process a received FPIN.
  * @shost:		host the FPIN was received on
@@ -642,6 +932,38 @@  EXPORT_SYMBOL(fc_host_post_vendor_event);
 void
 fc_host_fpin_rcv(struct Scsi_Host *shost, u32 fpin_len, char *fpin_buf)
 {
+	struct fc_els_fpin *fpin = (struct fc_els_fpin *)fpin_buf;
+	struct fc_tlv_desc *tlv;
+	u32 desc_cnt = 0, bytes_remain;
+	u32 dtag;
+
+	/* Update Statistics */
+	tlv = (struct fc_tlv_desc *)&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);
+		switch (dtag) {
+		case ELS_DTAG_LNK_INTEGRITY:
+			fc_fpin_li_stats_update(shost, tlv);
+			break;
+		case ELS_DTAG_DELIVERY:
+			fc_fpin_delivery_stats_update(shost, tlv);
+			break;
+		case ELS_DTAG_PEER_CONGEST:
+			fc_fpin_peer_congn_stats_update(shost, tlv);
+			break;
+		case ELS_DTAG_CONGESTION:
+			fc_fpin_congn_stats_update(shost, tlv);
+		}
+
+		desc_cnt++;
+		bytes_remain -= FC_TLV_DESC_SZ_FROM_LENGTH(tlv);
+		tlv = fc_tlv_next_desc(tlv);
+	}
+
 	fc_host_post_fc_event(shost, fc_get_event_number(),
 				FCH_EVT_LINK_FPIN, fpin_len, fpin_buf, 0);
 }
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index 487a403ee51e..a636c1986e22 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -819,6 +819,7 @@  void fc_host_post_event(struct Scsi_Host *shost, u32 event_number,
 		enum fc_host_event_code event_code, u32 event_data);
 void fc_host_post_vendor_event(struct Scsi_Host *shost, u32 event_number,
 		u32 data_len, char *data_buf, u64 vendor_id);
+struct fc_rport *fc_find_rport_by_wwpn(struct Scsi_Host *shost, u64 wwpn);
 void fc_host_post_fc_event(struct Scsi_Host *shost, u32 event_number,
 		enum fc_host_event_code event_code,
 		u32 data_len, char *data_buf, u64 vendor_id);