Bhanu Gollapudi wrote:
> Send fip solicitation if no advertisement is received for 2.5 * fka time, 
> instead of the current 3 * fka time, as per FC-BB-5 rev 2.

The fix is OK with me as is, but might be better if sol_time
wasn't added to struct fcoe_fcf.

Since fcf->sol_time can be computed from fcf->time, there's no
need to store it in the fcf.  We would have to compute it both
in age_fcfs() and in fcoe_ctlr_timeout() in order to set the
timer, so it might be good to write a short function to do it
or it could be returned from age_fcfs for the selected FCF.
I guess storing it, as you have, is OK.

Another thing I noticed is that the mod_timer() calls in age_fcfs()
really get overridden by the call in fcoe_ctlr_timeout(), so they're
not really necessary.  When I did that I must've planned to let
fcoe_ctlr_timeout() would reset it if it wanted it sooner.
Those extra mod_timer() calls are harmless, but should be removed.
Maybe that's another reason for age_fcfs() to return the time it
wants to be recalled and have fcoe_ctlr_timeout() use that for
its next_timer value.  I will do that separately if you don't
want to take care of it now.

> Signed-off-by: Bhanu Prakash Gollapudi <[email protected]>
> ---
>  drivers/scsi/fcoe/libfcoe.c |    7 +++++--
>  include/scsi/libfcoe.h      |    1 +
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c
> index 4aab7c3..d8a0d2d 100644
> --- a/drivers/scsi/fcoe/libfcoe.c
> +++ b/drivers/scsi/fcoe/libfcoe.c
> @@ -576,6 +576,8 @@ static void fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
>  
>       list_for_each_entry_safe(fcf, next, &fip->fcfs, list) {
>               mda_time = fcf->fka_period + (fcf->fka_period >> 1);
> +             fcf->sol_time = fcf->time + mda_time + fcf->fka_period +
> +                    msecs_to_jiffies(FIP_FCF_FUZZ * 3);
>               if ((fip->sel_fcf == fcf) &&
>                   (time_after(jiffies, fcf->time + mda_time))) {
>                       mod_timer(&fip->timer, jiffies + mda_time);
> @@ -587,8 +589,7 @@ static void fcoe_ctlr_age_fcfs(struct fcoe_ctlr *fip)
>                              fip->lp->host->host_no, fcf->fabric_name,
>                              stats->MissDiscAdvCount);
>               }
> -             if (time_after(jiffies, fcf->time + fcf->fka_period * 3 +
> -                            msecs_to_jiffies(FIP_FCF_FUZZ * 3))) {
> +             if (time_after(jiffies, fcf->sol_time)) {
>                       if (fip->sel_fcf == fcf)
>                               fip->sel_fcf = NULL;
>                       list_del(&fcf->list);
> @@ -1245,6 +1246,8 @@ static void fcoe_ctlr_timeout(unsigned long arg)
>               }
>               if (time_after(next_timer, fip->port_ka_time))
>                       next_timer = fip->port_ka_time;
> +             if (time_after(next_timer, sel->sol_time))
> +                     next_timer = sel->sol_time;
>               mod_timer(&fip->timer, next_timer);
>       } else if (fip->sel_time) {
>               next_timer = fip->sel_time +
> diff --git a/include/scsi/libfcoe.h b/include/scsi/libfcoe.h
> index ec13f51..7b8968d 100644
> --- a/include/scsi/libfcoe.h
> +++ b/include/scsi/libfcoe.h
> @@ -143,6 +143,7 @@ struct fcoe_ctlr {
>  struct fcoe_fcf {
>       struct list_head list;
>       unsigned long time;
> +     unsigned long sol_time;
>  
>       u64 switch_name;
>       u64 fabric_name;

Thanks for working on this.  It's good to see the improvements.

        Regards,
        Joe

_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel

Reply via email to