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

Zou, Yi yi.zou at intel.com
Wed Jan 19 20:11:53 UTC 2011


> 
> On Tue, 2011-01-18 at 18:02 -0800, Bhanu Gollapudi wrote:
> > 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.
> >
> 
> Great. I think we can still achieve the SW FCoE over a bnx2fc serviced
> port. If a user were to remove the bnx2fc driver it would unregister it
> as a transport and then when creating on the Ethernet interface it would
> default to fcoe.ko. There are certainly limitations with this approach,
> but I think it would work. For example, you wouldn't be able to have
> some ports bnx2fc.ko and some fcoe.ko, since you'd be rmmod'ing bnx2fc.
> Anyway, like you said, if there's a need we can increase the
> functionality later.
> 
So, I think we are on the same page of keeping the availability of match()
function and allow DRIVER_NAME to override. The netdev map is still needed
as Bhanu pointed out, so destroy would always go to netdev map to look up for
the transport. I will start making changes to reflect what we have discussed.

Thanks,
yi


> 
> _______________________________________________
> devel mailing list
> devel at open-fcoe.org
> https://lists.open-fcoe.org/mailman/listinfo/devel



More information about the devel mailing list