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@xxxxxxxxxxxx> wrote:
On 10/5/2020 11:16 PM, Nilesh Javali wrote:
> From: Shyam Sundar <ssundar@xxxxxxxxxxx>
>
> Parse the incoming FPIN packets and update the host and rport FPIN
> statistics based on the FPINs.
>
> Signed-off-by: Shyam Sundar <ssundar@xxxxxxxxxxx>
> Signed-off-by: Nilesh Javali <njavali@xxxxxxxxxxx>
> ---
> ...
> +/*
> + * 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
