All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Smart <james.smart@broadcom.com>
To: Shyam Sundar <ssundar@marvell.com>
Cc: Nilesh Javali <njavali@marvell.com>,
	"Martin K . Petersen" <martin.petersen@oracle.com>,
	GR-QLogic-Storage-Upstream
	<GR-QLogic-Storage-Upstream@marvell.com>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	Arun Easi <aeasi@marvell.com>
Subject: Re: [PATCH 2/3] qla2xxx: SAN congestion management(SCM) implementation.
Date: Thu, 25 Jun 2020 16:25:31 -0700	[thread overview]
Message-ID: <7c33c4fb-d3ec-cd2e-4de3-ecb95ffec8b8@broadcom.com> (raw)
In-Reply-To: <351333B3-F666-420F-A9D3-DB86D2617156@marvell.com>



On 6/11/2020 10:42 AM, Shyam Sundar wrote:
> Seems like this (and a previous email) never made it to the reflector, resending.
>
> The suggestions make sense to me, and I have made most of the recommended changes.
> I was looking for some guidance on placements of the sim stats structures.
>
>> On May 29, 2020, at 11:53 AM, Shyam Sundar <ssundar@marvell.com> wrote:
>>
>> James,
>>       I was thinking of adding a structures for tracking the target FPIN stats.
>>
>> struct fc_rport_statistics {
>> uint32_t link_failure_count;
>> uint32_t loss_of_sync_count;
>> ....
>> }
>>
>> under fc_rport:
>>
>> struct fc_rport {
>>
>> /* Private (Transport-managed) Attributes */
>> struct fc_rport_statistics;
>>
>>       For host FPIN stats (essentially the alarm & warning), was not sure if I should add them to the fc_host_statistics or
>> define a new structure under the Private Attributes section within the fc_host_attrs/fc_vport.

my initial thought was the same

>>
>>       In theory, given that the host stats could be updated both via signals and FPIN, one could argue that it would make sense
>> to maintain it with the current host statistics, but keeping it confined to transport will ensure we have a uniform way of handling
>> congestion and peer congestion events.

but then I have the same thoughts as well. And the commonization of the 
parsing and incrementing makes a lot more sense.

>>
>>      Would appreciate your thoughts there.
>>
>> Thanks
>> Shyam
>>     
>>


I would put them under the Dynamic attributes area in fc_host_attrs and 
fc_rport.
fc_host_attrs:
fpin_cn              incremented for each Congestion Notify FPIN
cn_sig_warn     incremented for each congestion warning signal
cn_sig_alarm    incremented for each congestion alarm signal
fpin_dn             incremented for each Delivery Notification FPIN 
where attached wwpn is local port
fpin_li                incremented for each Link Integrity FPIN where 
attached wwpn is local port

fc_rport:
fpin_dn             incremented for each Delivery Notification FPIN 
where attached wwpn is the rport
fpin_li               incremented for each Link Integrity FPIN where 
attached wwpn is the rport
fpin_pcn           incremented for each Peer Congestion Notify FPIN 
where attached wwpn is the rport

For the cn_sig_xxx values, the driver would just increment them.
For the fpin_xxx values - we'll augment the fc_host_fpin_rcv routine to 
parse the fpin and increment the fc_host or fc_rport counter.

-- james



  reply	other threads:[~2020-06-25 23:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-14 10:10 [PATCH 0/3] qla2xxx SAN Congestion Management (SCM) support Nilesh Javali
2020-05-14 10:10 ` [PATCH 1/3] qla2xxx: Change in PUREX to handle FPIN ELS requests Nilesh Javali
2020-05-14 17:03   ` himanshu.madhani
2020-05-15 18:52   ` James Smart
2020-05-14 10:10 ` [PATCH 2/3] qla2xxx: SAN congestion management(SCM) implementation Nilesh Javali
2020-05-14 18:52   ` himanshu.madhani
2020-05-15 22:48   ` James Smart
     [not found]     ` <CA+ihqdiA7AN05k5MjPG=o8_pf=L-La6UigY4t0emKgJMXm=hnQ@mail.gmail.com>
     [not found]       ` <BYAPR18MB2805AEA357302FCFA20D2B57B48F0@BYAPR18MB2805.namprd18.prod.outlook.com>
2020-06-11 17:42         ` Shyam Sundar
2020-06-25 23:25           ` James Smart [this message]
2020-06-26  0:14             ` Shyam Sundar
2020-07-30 16:10             ` Shyam Sundar
2020-09-21 17:48               ` James Smart
     [not found]     ` <CA+ihqdjtoA=1q7N0pg1TQDAMGo1XtNN8+XnO1qXORyqGYfpq=A@mail.gmail.com>
2020-06-11 18:10       ` Shyam S
2020-05-14 10:10 ` [PATCH 3/3] qla2xxx: Pass SCM counters to the application Nilesh Javali
2020-05-14 19:15   ` himanshu.madhani
2020-05-14 14:11 ` [PATCH 0/3] qla2xxx SAN Congestion Management (SCM) support Bart Van Assche

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7c33c4fb-d3ec-cd2e-4de3-ecb95ffec8b8@broadcom.com \
    --to=james.smart@broadcom.com \
    --cc=GR-QLogic-Storage-Upstream@marvell.com \
    --cc=aeasi@marvell.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=njavali@marvell.com \
    --cc=ssundar@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.