[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 22:47:16 UTC 2011


> On Wed, 2011-01-19 at 12:11 -0800, Zou, Yi wrote:
> > >
> > > 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.
> 
> Yi,
> 
> As per our discussions y'day, we decided to not have DRIVER_NAME change
> now, and if this ever becomes a requirement, we will make the change.
> 
> For now, there will not be any fcoeutils change, and the parse buffer
> change in the driver.
Ok, but I don't see you guys have a match() in your transport in your
V3 patch set:
 https://lists.open-fcoe.org/pipermail/devel/2011-January/011011.html

am I missing anything here?


> 
> But, I think we should retain the SUPPORTED_DRIVERS. If there is no
> objection, we will submit a seperate patch for it.
Cool, sounds good to me.

Thanks,
yi

> 
> Thanks,
> Bhanu
> >
> > 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