[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 22:16:35 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.

But, I think we should retain the SUPPORTED_DRIVERS. If there is no
objection, we will submit a seperate patch for it.

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