[Open-FCoE] [PATCH v2] fcoe-utils: add DRIVER_NAME to specify the FCoE low-level driver

Bhanu Gollapudi bprakash at broadcom.com
Wed Jan 19 02:02:22 UTC 2011


On Tue, 2011-01-18 at 17:42 -0800, Robert Love wrote:
> On Tue, 2011-01-18 at 17:10 -0800, Bhanu Gollapudi wrote:
> > On Tue, 2011-01-18 at 15:52 -0800, Robert Love wrote:
> > > > 
> > > > Also, I'd really prefer a way that the user didn't have to choose the
> > > > driver, since I think it makes the solution more user-friendly, maybe
> > > > this is something we can evolve towards.
> > > > 
> > > 
> > > I've been thinking about this more and I really dislike that the user
> > > has to specify the driver name. Yi's first fcoe transport series was
> > > using a "match" routine in the kernel to select which fcoe transport to
> > > use. Here is a snippit:
> > > 
> > > +/**
> > > + * fcoe_transport_lookup - find an fcoe transport that supports the
> > > netdev
> > > + * @ft: The fcoe transport to be attached
> > > + *
> > > + * Returns : ptr to the fcoe transport that supports this netdev or
> > > NULL
> > > + * if not found.
> > > + *
> > > + * The ft_mutex should be held when this is called
> > > + */
> > > +static struct fcoe_transport *fcoe_transport_lookup(struct net_device
> > > *netdev)
> > > +{
> > > +	struct fcoe_transport *ft;
> > > +
> > > +	list_for_each_entry(ft, &fcoe_transports, list)
> > > +		if (ft->match && ft->match(netdev))
> > > +			goto ft_match;
> > > +	ft = NULL;
> > > +ft_match:
> > > +	return ft;
> > > +}
> > > 
> > > I like this because the user doesn't have to make any changes to the
> > > config files for a new driver. All we need to do is have an in-kernel
> > > match routine for bnx2fc (or any other fcoe driver). The "create" entry
> > > point would only receive the ifname. libfcoe would then go through the
> > > list of registered transports, calling match() on each to find which
> > > driver should be used with the provided interface's netdev instance.
> > > We'd need to put any fcoe drivers first in the match list and have the
> > > default always be fcoe.ko.
> > > 
> > > To accomplish this we would just need to update Yi's series with this
> > > transport lookup/match code, we'd need to have a match() routine for
> > > bnx2fc and we wouldn't need to make any changes to fcoe-utils.
> > 
> > We do not have any objections with the transport lookup/match routine,
> > but would like to raise the following points:
> > 
> > 1. transport lookup/match() should only be called in create, and
> > destroy/disable/enable should lookup the netdev  map (as done today).
> > This is to avoid a situation where create is called on one driver, and
> > destroy is called on the other. For eg., initially fcoe driver claims
> > interface X, and then bnx2fc is loaded, which claims the same interface.
> > Now, when destroy is called, we would end up calling into
> > bnx2fc_destroy(). netdev map logic fixed this issue, and we should
> > retain it.
> > 
> > 2. We will lose the flexibility where a user can choose which driver to
> > run on a particular interface by editing the DRIVER_NAME.  For eg., if
> > bnx2fc is loaded, all the Broadcom netxtreme2 FCoE licenced interfaces
> > will be claimed by bnx2fc, and customer cannot run software FCoE while
> > bnx2fc is loaded on these interfaces.  It is not a requirement. We are
> > okay with it, but wanted to bring this up.
> > 
> 
> Yes, this is true. I'm OK with keeping the DRIVER_NAME as a way to
> override the match function. That way you can force SW FCoE on an
> interface that would normally be serviced by bnx2fc. I think this could
> be helpful if debugging. If we do this I think it will be important to
> have a clear kernel message that the default driver is being overridden.
> Also, I think we should use a stronger variable name, like FORCE_DRIVER
> and make the fcoeadm option something like '--force-driver', just so the
> user really understands that they're doing something out of the
> ordinary.

Lets keep it simple by not changing the fcoeutils for now. If this
becomes a requirement we will follow this approach.

> 
> 
> 






More information about the devel mailing list