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

Zou, Yi yi.zou at intel.com
Thu Jan 20 00:36:49 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?
> 
Just want to add that, I will make fcoe.ko as the default by adding to
the tail of transport list, so any vendor transport can claim the netdev
if it matches up since the lookup go through vendor transports first, 
where fcoe.ko's match() function will return *always* true, and the 
name "fcoe" will be reserved for default transport driver fcoe.ko.

Thanks,
Yi

> 
> >
> > 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
> > >
> >
> >
> 
> _______________________________________________
> devel mailing list
> devel at open-fcoe.org
> https://lists.open-fcoe.org/mailman/listinfo/devel



More information about the devel mailing list