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

Robert Love robert.w.love at intel.com
Tue Jan 18 23:52:41 UTC 2011


On Tue, 2011-01-18 at 11:52 -0800, Robert Love wrote:
> On Mon, 2011-01-17 at 18:22 -0800, Bhanu Gollapudi wrote:
> > On Mon, 2011-01-17 at 17:35 -0800, Robert Love wrote:
> > > On Wed, 2011-01-12 at 18:42 -0800, Nithin Sujir wrote:
> > > > This patch adds support to fcoe-utils to send the low-level driver name into
> > > > the libfcoe driver. This complements yi's driver v3 patches that change the
> > > > sysfs path and add the transport attach support. 
> > > > 
> > > > https://lists.open-fcoe.org/pipermail/devel/2011-January/010946.html
> > > > 
> > > > The following changes are added.
> > > > 
> > > > 1. DRIVER_NAME field in cfg-ethx file. fcoemon will now send "interface:driver
> > > > name" string into the libfcoe driver to allow it to invoke the right transport
> > > > driver functions.
> > > > 
> > > > 2. SUPPORTED_DRIVERS field in the config file allows the service to load all
> > > > supported drivers if they exist.
> > > > 
> > > > 3. The check for if driver is loaded is moved to the fcoe service since it
> > > > already knows which are the supported drivers.
> > > > 
> > > > 
> > > > Signed-off-by: Nithin Nayak Sujir <nsujir at broadcom.com>
> > > > ---
> > > >  doc/fcoemon.txt        |    3 ++
> > > >  etc/cfg-ethx           |    5 ++++
> > > >  etc/config             |    4 +++
> > > >  etc/initd/initd.fedora |   23 +++++++++++++++++-
> > > >  etc/initd/initd.suse   |   25 ++++++++++++++++++-
> > > >  fcoemon.c              |   59 +++++++++++++++++++++++++++++++++++-------------
> > > >  include/fcoe_utils.h   |    4 ++-
> > > >  7 files changed, 102 insertions(+), 21 deletions(-)
> > > > 
> > > 
> > > Hi Nithin, Bhanu and Yi,
> > > 
> > >    We need an update to the fcoeadm man page, can you resend this with
> > > that addition?
> > 
> > Hi Robert,
> > 
> > There are no changes to fcoadm man page, since none of the options have
> > changed.
> > 
> > > 
> > >    Also, I was trying to test Yi/Bhanu's v3 kernel series with this
> > > fcoe-utils patch and I can't get things to work.
> > > 
> > >    After running 'fipvlan -ac' interface 'eth3.170-fcoe' is created.
> > > When I try to create on that interface using 'fcoeadm -c eth3.170-fcoe'
> > > nothing happens except for the kernel reporting, "fcoe_transport_create:
> > > transport n/a failed to create fcoe on n/a"
> > 
> > Shouldn't we pass physical ethernet interface (eth4) instead of the vlan
> > interface?
> > 
> 
> Possibly. Passing the physical interface to the kernel would mean that
> we'd need to do VLAN discovery in the kernel. Since the general kernel
> attitude is to do as much as possible in user space and the fact that
> the current solution seems to work fine, nobody has tried to push VLAN
> discovery into the kernel.
> 
> I would be fine with moving LVAN discovery into the kernel if we come
> across a good reason- so far I'm not aware of one.
> 
> > I tried similar call 'fcoeadm -c eth4.4-fcoe', but it gave the following
> > error:
> > 'fcoeadm: Connection already created on interface eth4.4-fcoe'. I think
> > this is a default error message when fcoemon fails to create.
> > 
> 
> I hit this a few times when I was testing too. I think it was because I
> had a mismatch of fcoe-utils an kernel modules. This is such a
> misleading error message though, we need to fix it.
> 
> > 
> > > When I try 'fcoeadm -c eth3.170-fcoe:fcoe' I get the following system
> > > panic.
> > > 
> > >    With fcoe.ko how are you expecting the user to use 'fcoeadm -c', with
> > > the ':<driver>' or not? 
> > We expect user to use 'fcoeadm -c' without a :<driver>
> > 
> 
> How will the bnx2fc driver be selected if there is no config file? Also,
> with this patch if the user doesn't have the foresight to edit the
> DRIVER_NAME before starting fcoemon then they're stuck with the default.
> 
> Just a few talking points:
> 
> We have talked about adding a 'save' option to fcoeadm, so that any
> non-persistent connections will be written to new config files. With
> this functionality you could do 'fcoeadm -c ethX.vlan-fcoe:foo' and then
> 'fcoeadm --save' and you'd then have a persistent connection on that
> interface using the foo driver.
> 
> We've also talked about adding 'reload' functionality to the fcoe
> service. This would have fcoemon re-read the config files, check for
> differences from when it started, and take action on any of those
> differences. Assuming the user starts the fcoe service without
> specifying the correct driver, he could update the DRIVER_NAME=foo
> and then reload the service to recreate the connection using the correct
> driver.
> 
> Either of those two solutions would help so that you don't need to
> 'restart' the fcoe service to bring up LUNs on a new interface. The
> reason you don't want to 'restart' is because it will tear-down and
> recreate any existing logins to the fabric.
> 
> 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.

The only thing that I don't like about this is that the fcoe service
will still need to load all fcoe drivers when it starts since they need
to be loaded to register as fcoe transports. I can't think of a way
around this at this point.

Thanks, //Rob




More information about the devel mailing list