Re: [PATCH 2/3] qla2xxx: SAN congestion management(SCM) implementation.

[James Smart <james.smart@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 4382 bytes --]

Shyam,

comments will be posted shortly

-- james


On 7/30/2020 9:10 AM, Shyam Sundar wrote:
> James,
>                                                                                  
> Please review the updated patchset to see if the changes make sense to you.
> There are a few deviations from what we discussed here.
>                                                                                  
> I have used a single structure definition to maintain all fpin statistics
> and we are maintain them at the granularity provided by the fabric instead of
> Using a single counter per category. The same
> structure is used across the fc_host_attr and the fc_rport structures.
>                                                                                  
> Given that the congestion signals will have to be maintained by the LLDs,
> as we do not have the plumbing to push those notifications to the transport,
> I have added them to the fc_host_statistics maintained and filled in by the
> LLD. All of the stats are displayed under the "statistics" folder under the
> sysfs.
>                                                                                  
> Under the host stats, a subset of stats (the Link Integrity FPINs) are
> redundunt, and are already maintained, read via the traditional mechanism.
> I have rolled them in nonetheless because during testing, they did not
> update to the same values (using the FPIN error injection commands). If we
> determine that these values will be in sync consistently, we can remove one
> of the subset at a later point.
>                                                                                  
> Regards
> Shyam
>
>> On Jun 25, 2020, at 4:25 PM, James Smart <james.smart@broadcom.com> wrote:
>>
>>
>>
>> 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


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4163 bytes --]