[Open-FCoE] [PATCH 13/13] fcoe-utils: Ignore interface operational state

Joe Eykholt jeykholt at cisco.com
Wed Jul 8 00:54:43 UTC 2009


Robert Love wrote:
> Currently fcoemon has a set of criteria that it uses to determine
> if an interface should be created or destroyed. The current set
> includes the link state (i.e. operational state). What this means
> is that fcoemon will not try to create the interface if there is
> no link. It also means that if the link goes down on an already
> created interface then it will destroy that interface.
> 
> The libfc/fcoe protocol stack in the kernel already handles link
> events. Generally, on link down the stack blocks I/O and on link
> up it un-blocks I/O. So, if the Ethernet cable is pulled out, I/O
> will pause. If that same cable is re-inserted then the I/O will
> resume.
> 
> The problem is that fcoemon is interfering with the kernel's
> ability to resume I/O. This is happening becuase fcoemon will
> receive the link down event. It will see that it does not meet
> it's criteria and it will destory the interface. This destory
> frees all of the kernel's per-instance data for that interface
> and when the link becomes active again the I/O cannot be restarted
> becuase the required structures have been freed.
> 
> This patch removes the 'operational state' criteria from the
> set of requirements fcoemon uses when determining whether to
> create or destroy an interface.
> 
> Interesting notes:
> 
> This patch does not modify the link notification code at all and
> we are still caching the link state.
> 
> Also, link down events will generate a few dcbd operational change
> event. None of these events alters the behavior of fcoemon (regarding
> create/destroy of interfaces) so this should be fine.


I agree with all this and this patch set is probably fine
as far as it goes.  (I haven't reviewed it entirely).
I think more is needed, though.

There should be some interaction between fcoemon and fcoe
concerning pause.

If the interface doesn't have PFC (priority flow
control) or link pause enabled, it shouldn't be used for I/O.
The usual symptom will be writes never finishing because we
send data too fast and some of it gets dropped.

So, we might want to add a /sys variable that indicates that
PFC is present and fcoeplumb has been done, etc.  That would
get cleared by fcoe when the link goes down, and set by fcoe
if link-pause is present, or set by fcoeplumb or fcoemon
when PFC is set up.

I probably said all this before ... at least somewhere.

	Joe



> Signed-off-by: Robert Love <robert.w.love at intel.com>
> ---
> 
>  fcoemon.c |   18 ------------------
>  1 files changed, 0 insertions(+), 18 deletions(-)
> 
> diff --git a/fcoemon.c b/fcoemon.c
> index 529056d..d42c39b 100644
> --- a/fcoemon.c
> +++ b/fcoemon.c
> @@ -660,16 +660,6 @@ static void fcm_fcoe_set_name(struct fcm_fcoe *ff, char *ifname)
>  	ff->ff_vlan = vlan;
>  }
>  
> -static int fcm_fcoe_port_ready(struct fcm_fcoe *ff)
> -{
> -	int rc;
> -
> -	rc = (ff->ff_flags & (IFF_UP|IFF_RUNNING)) == (IFF_UP|IFF_RUNNING) &&
> -		ff->ff_operstate == IF_OPER_UP;
> -
> -	return rc;
> -}
> -
>  static void fcm_dcbd_init()
>  {
>  	fcm_clif->cl_fd = -1;	/* not connected */
> @@ -1732,19 +1722,11 @@ static void fcm_dcbd_port_advance(struct fcm_fcoe *ff)
>  	if (!p)
>  		return;
>  
> -	if (ff->ff_dcbd_state != FCD_INIT && !fcm_fcoe_port_ready(ff))
> -		fcm_dcbd_state_set(ff, FCD_INIT);
>  	if (fcm_clif->cl_busy)
>  		return;
>  
>  	switch (ff->ff_dcbd_state) {
>  	case FCD_INIT:
> -		if (!fcm_fcoe_port_ready(ff)) {
> -			if (fcm_debug)
> -				SA_LOG("FCoE port %s not ready\n", ff->ff_name);
> -			fcm_dcbd_state_set(ff, FCD_ERROR);
> -			break;
> -		}
>  		fcm_dcbd_state_set(ff, FCD_GET_DCB_STATE);
>  		/* Fall through */
>  	case FCD_GET_DCB_STATE:
> 
> _______________________________________________
> devel mailing list
> devel at open-fcoe.org
> http://www.open-fcoe.org/mailman/listinfo/devel




More information about the devel mailing list