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 statisticsand 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) areredundunt, 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.RegardsShyamOn Jun 25, 2020, at 4:25 PM, James Smart <james.smart@xxxxxxxxxxxx> 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@xxxxxxxxxxx> 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 sameIn 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 ShyamI 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:
smime.p7s
Description: S/MIME Cryptographic Signature
