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

[Shyam Sundar <ssundar@marvell.com>]

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