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

Zou, Yi yi.zou at intel.com
Thu Jan 6 21:52:21 UTC 2011


> 
> On 1/5/11 6:00 PM, Zou, Yi wrote:
> >>
> >> This patch adds support to fcoe-utils to send the low-level driver
> name
> >> into
> >> the libfcoe driver. This complements yi's recent driver patches that
> >> change the
> >> sysfs path and add the transport attach support.
> >>
> >> 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.
> >>
> > We need a way to be backward compatible, or at least somehow let the
> user
> > know if the fcoe-utils is not matching up w/ the running kernel on
> this.
> 
> Agreed.  It's easy enough for the script to see if the parameter is
> in /sys/module/libfcoe or fcoe.
> 
> Also, could someone explain why its necessary to specify the driver name?
> It's still an Ethernet device, right?  Is it because offloads are
> different
> or what?  Is it possible for fcoe/libfcoe to query the ethernet device
> driver directly to find out what it is?  I must be missing something
> obvious.
> 
> Sorry for the late comments.  I've been out.
> 
> 	Joe
> 
To *my* understanding, the driver name is for the vendor specific fcoe hba
driver acting as the fcoe transport, effectively replacing the existing sw
fcoe driver (fcoe.ko) to take the i/o path directly into the hardware. This is
not the actual network driver, which I assume does things other than fcoe
traffic, though it refers to the network interface so it fits nicely into the
existing open-fcoe framework.

Using driver name seems simpler to me, though I originally proposed using the
fcoe transport's match() function, which is expected to be implemented by the
vendor to tell if this netdev belongs to its fcoe transport since you can get
to the parent pci device from the netdev and match by vendor/device id etc. Both
would need the fcoe transport or anything similar, w/o which I am not sure what
best way there is to find out that information.

Related threads are:
https://lists.open-fcoe.org/pipermail/devel/2010-December/010865.html
https://lists.open-fcoe.org/pipermail/devel/2011-January/010890.html

This kernel change makes it necessary to change the fcoe-utils to point to the
libfcoe for create/destroy etc. as the previous patch on fcoe-util.

thanks,

yi

> >
> >>
> >> 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(-)
> >>
> >> diff --git a/doc/fcoemon.txt b/doc/fcoemon.txt
> >> index f42c8c0..f69f0f6 100644
> >> --- a/doc/fcoemon.txt
> >> +++ b/doc/fcoemon.txt
> >> @@ -187,6 +187,9 @@ _AUTO_VLAN_::
> >>  	interfaces.  If the network interface specified by the filename is
> >>  	already a VLAN interface, the AUTO_VLAN setting is ignored.
> >>
> >> +_DRIVER_NAME_::
> >> +	indicates the name of the driver that will claim this interface.
> >> +
> >>  Note that the attached Ethernet peer device (e.g. FCoE capable switch
> >> port)
> >>  must have compatible settings For DCB and FCoE to function properly.
> >>
> >> diff --git a/etc/cfg-ethx b/etc/cfg-ethx
> >> index b7274ac..0616bb4 100644
> >> --- a/etc/cfg-ethx
> >> +++ b/etc/cfg-ethx
> >> @@ -12,3 +12,8 @@ DCB_REQUIRED="yes"
> >>  ## Default:	yes
> >>  # Indicate if VLAN discovery should be handled by fcoemon
> >>  AUTO_VLAN="yes"
> >> +
> >> +## Type:	string
> >> +## Default:	fcoe
> >> +# The name of the driver that will claim this interface
> >> +DRIVER_NAME="fcoe"
> >> diff --git a/etc/config b/etc/config
> >> index 2d08c9e..c993f35 100644
> >> --- a/etc/config
> >> +++ b/etc/config
> >> @@ -8,3 +8,7 @@ DEBUG="no"
> >>  # All the messages go to syslog and stderr (script & C code)
> >>  USE_SYSLOG="yes"
> >>
> >> +## Type:       string. Driver names separated by space
> >> +## Default:    list of default drivers
> >> +# All supported drivers listed here are loaded when service starts
> >> +SUPPORTED_DRIVERS="fcoe bnx2fc"
> > It will be nice if fcoeadm is able to enable/disable vendor and update
> > the config...but I guess it's ok for now.
> >
> > Thanks,
> > yi
> >
> >> diff --git a/etc/initd/initd.fedora b/etc/initd/initd.fedora
> >> index 3888dda..56a883e 100755
> >> --- a/etc/initd/initd.fedora
> >> +++ b/etc/initd/initd.fedora
> >> @@ -63,14 +63,33 @@ test -x $FCOEMON || {
> >>  	fi
> >>  }
> >>
> >> +check_drivers()
> >> +{
> >> +	driver_loaded=no
> >> +
> >> +	for driver in $SUPPORTED_DRIVERS; do
> >> +		if [ -d /sys/module/$driver/parameters ]; then
> >> +			driver_loaded=yes
> >> +		fi
> >> +	done
> >> +
> >> +	if [ "$driver_loaded" = "yes" ]; then
> >> +		daemon --pidfile ${PID_FILE} ${FCOEMON} ${FCOEMON_OPTS}
> >> +	else
> >> +		echo
> >> +		echo "No supported drivers loaded"
> >> +		failure
> >> +	fi
> >> +}
> >> +
> >>  start()
> >>  {
> >>  	echo -n $"Starting FCoE initiator service: "
> >>
> >>  	modprobe -q libfc
> >> -	modprobe -q fcoe
> >> +	modprobe -q -a $SUPPORTED_DRIVERS
> >>
> >> -	daemon --pidfile ${PID_FILE} ${FCOEMON} ${FCOEMON_OPTS}
> >> +	check_drivers
> >>
> >>  	echo
> >>  	touch /var/lock/subsys/fcoe
> >> diff --git a/etc/initd/initd.suse b/etc/initd/initd.suse
> >> index 139de2d..7deba1e 100755
> >> --- a/etc/initd/initd.suse
> >> +++ b/etc/initd/initd.suse
> >> @@ -82,7 +82,28 @@ test -x $FCOEMON || {
> >>
> >>  startup_fcoe_modules()
> >>  {
> >> -	modprobe fcoe > /dev/null 2>&1
> >> +	modprobe -a $SUPPORTED_DRIVERS > /dev/null 2>&1
> >> +}
> >> +
> >> +check_drivers()
> >> +{
> >> +	driver_loaded=no
> >> +
> >> +	for driver in $SUPPORTED_DRIVERS; do
> >> +		if [ -d /sys/module/$driver/parameters ]; then
> >> +			driver_loaded=yes
> >> +		fi
> >> +	done
> >> +
> >> +	if [ "$driver_loaded" = "yes" ]; then
> >> +		startproc -l ${LOG_FILE} -p ${PID_FILE} ${FCOEMON}
> >> ${FCOEMON_OPTS}
> >> +	else
> >> +		echo
> >> +		echo "No supported drivers loaded"
> >> +		rc_failed
> >> +		rc_status -v
> >> +		rc_exit
> >> +	fi
> >>  }
> >>
> >>  start()
> >> @@ -90,8 +111,8 @@ start()
> >>  	echo -n $"Starting FCoE initiator service: "
> >>
> >>  	startup_fcoe_modules
> >> +	check_drivers
> >>
> >> -	startproc -l ${LOG_FILE} -p ${PID_FILE} ${FCOEMON} ${FCOEMON_OPTS}
> >>
> >>  	rc_status -v
> >>  }
> >> diff --git a/fcoemon.c b/fcoemon.c
> >> index bf0de93..fe73bbc 100644
> >> --- a/fcoemon.c
> >> +++ b/fcoemon.c
> >> @@ -124,6 +124,8 @@ struct fcoe_port {
> >>  	struct sa_timer vlan_disc_timer;
> >>  	int vlan_disc_count;
> >>  	int fip_socket;
> >> +
> >> +	char driver_name[MAX_DRV_NAME_LEN];
> >>  };
> >>
> >>  enum fcoeport_ifname {
> >> @@ -143,7 +145,7 @@ static void fcm_dcbd_event(char *, size_t);
> >>  static void fcm_dcbd_cmd_resp(char *, cmd_status);
> >>  static void fcm_netif_advance(struct fcm_netif *);
> >>  static void fcm_fcoe_action(struct fcm_netif *, struct fcoe_port *);
> >> -static int fcm_fcoe_if_action(char *, char *);
> >> +static int fcm_fcoe_if_action(char *, char *, char *);
> >>
> >>  struct fcm_clif {
> >>  	int cl_fd;
> >> @@ -372,6 +374,27 @@ static int fcm_read_config_files(void)
> >>  		if (!strncasecmp(val, "yes", 3) && rc == 1)
> >>  			next->dcb_required = 1;
> >>
> >> +
> >> +		/* DRIVER_NAME */
> >> +		rc = fcm_read_config_variable(file, val, sizeof(val),
> >> +					      fp, "DRIVER_NAME");
> >> +		if (rc < 0) {
> >> +			FCM_LOG("%s invalid format for DRIVER_NAME setting");
> >> +			fclose(fp);
> >> +			free(next);
> >> +			continue;
> >> +		}
> >> +		/* if DRIVER_NAME not found, error out */
> >> +		if (rc == 1) {
> >> +			FCM_LOG("DRIVER_NAME found %s", val);
> >> +			strncpy(next->driver_name, val, MAX_DRV_NAME_LEN - 1);
> >> +		} else {
> >> +			FCM_LOG("DRIVER_NAME not found");
> >> +			fclose(fp);
> >> +			free(next);
> >> +			continue;
> >> +		}
> >> +
> >>  		/* AUTO_VLAN */
> >>  		rc = fcm_read_config_variable(file, val, sizeof(val),
> >>  					      fp, "AUTO_VLAN");
> >> @@ -552,6 +575,8 @@ int fcm_vlan_disc_handler(struct fiphdr *fh,
> struct
> >> sockaddr_ll *sa, void *arg)
> >>  			vid = ntohs(((struct fip_tlv_vlan *)tlv)->vlan);
> >>  			vp = fcm_new_vlan(sa->sll_ifindex, vid);
> >>  			vp->dcb_required = p->dcb_required;
> >> +			strncpy(vp->driver_name, p->driver_name,
> >> +					MAX_DRV_NAME_LEN - 1);
> >>  			break;
> >>  		default:
> >>  			/* unexpected or unrecognized descriptor */
> >> @@ -1279,7 +1304,8 @@ static void fcm_cleanup(void)
> >>
> >>  	for (curr = fcoe_config.port; curr; curr = next) {
> >>  		FCM_LOG_DBG("OP: DESTROY %s\n", curr->ifname);
> >> -		fcm_fcoe_if_action(FCOE_DESTROY,  curr->ifname);
> >> +		fcm_fcoe_if_action(FCOE_DESTROY,  curr->ifname,
> >> +							curr->driver_name);
> >>  		next = curr->next;
> >>  		free(curr);
> >>  	}
> >> @@ -1959,10 +1985,17 @@ static void fcm_cli_reply(struct sock_info *r,
> >> int status)
> >>  			r->fromlen);
> >>  }
> >>
> >> -static int fcm_fcoe_if_action(char *path, char *ifname)
> >> +static int fcm_fcoe_if_action(char *path, char *ifname, char
> >> *driver_name)
> >>  {
> >>  	FILE *fp = NULL;
> >>  	int ret = fcm_fail;
> >> +	char buf[MAX_STR_LEN];
> >> +	strncpy(buf, ifname, MAX_STR_LEN - 1);
> >> +
> >> +	if (driver_name)
> >> +		snprintf(buf, MAX_STR_LEN - 1, "%s:%s", ifname, driver_name);
> >> +	else
> >> +		strncpy(buf, ifname, MAX_STR_LEN - 1);
> >>
> >>  	fp = fopen(path, "w");
> >>  	if (!fp) {
> >> @@ -1971,7 +2004,7 @@ static int fcm_fcoe_if_action(char *path, char
> >> *ifname)
> >>  		goto err_out;
> >>  	}
> >>
> >> -	if (EOF == fputs(ifname, fp)) {
> >> +	if (EOF == fputs(buf, fp)) {
> >>  		FCM_LOG_ERR(errno, "%s: Failed to write %s to path %s.\n",
> >>  				progname, ifname, path);
> >>  		goto out;
> >> @@ -2035,7 +2068,7 @@ static void fcm_fcoe_action(struct fcm_netif
> *ff,
> >> struct fcoe_port *p)
> >>  	switch (p->action) {
> >>  	case FCP_CREATE_IF:
> >>  		FCM_LOG_DBG("OP: CREATE %s\n", p->ifname);
> >> -		rc = fcm_fcoe_if_action(FCOE_CREATE, ifname);
> >> +		rc = fcm_fcoe_if_action(FCOE_CREATE, ifname, p->driver_name);
> >>  		break;
> >>  	case FCP_DESTROY_IF:
> >>  		FCM_LOG_DBG("OP: DESTROY %s\n", p->ifname);
> >> @@ -2050,11 +2083,11 @@ static void fcm_fcoe_action(struct fcm_netif
> *ff,
> >> struct fcoe_port *p)
> >>  			rc = fcm_success;
> >>  			break;
> >>  		}
> >> -		rc = fcm_fcoe_if_action(FCOE_DESTROY, ifname);
> >> +		rc = fcm_fcoe_if_action(FCOE_DESTROY, ifname, p-
> >>> driver_name);
> >>  		break;
> >>  	case FCP_ENABLE_IF:
> >>  		FCM_LOG_DBG("OP: ENABLE %s\n", p->ifname);
> >> -		rc = fcm_fcoe_if_action(FCOE_ENABLE, ifname);
> >> +		rc = fcm_fcoe_if_action(FCOE_ENABLE, ifname, p->driver_name);
> >>  		break;
> >>  	case FCP_DISABLE_IF:
> >>  		FCM_LOG_DBG("OP: DISABLE %s\n", p->ifname);
> >> @@ -2068,7 +2101,7 @@ static void fcm_fcoe_action(struct fcm_netif
> *ff,
> >> struct fcoe_port *p)
> >>  			}
> >>  			break;
> >>  		}
> >> -		rc = fcm_fcoe_if_action(FCOE_DISABLE, ifname);
> >> +		rc = fcm_fcoe_if_action(FCOE_DISABLE, ifname, p-
> >>> driver_name);
> >>  		break;
> >>  	case FCP_RESET_IF:
> >>  		FCM_LOG_DBG("OP: RESET %s\n", p->ifname);
> >> @@ -2083,7 +2116,7 @@ static void fcm_fcoe_action(struct fcm_netif
> *ff,
> >> struct fcoe_port *p)
> >>  		}
> >>
> >>  		sprintf(path, "%s/%s/issue_lip", SYSFS_FCHOST, fchost);
> >> -		rc = fcm_fcoe_if_action(path, "1");
> >> +		rc = fcm_fcoe_if_action(path, "1", NULL);
> >>  		break;
> >>  	case FCP_SCAN_IF:
> >>  		FCM_LOG_DBG("OP: SCAN %s\n", p->ifname);
> >> @@ -2099,7 +2132,7 @@ static void fcm_fcoe_action(struct fcm_netif
> *ff,
> >> struct fcoe_port *p)
> >>
> >>  		sprintf(path, "%s/%s/device/scsi_host/%s/scan",
> >>  			SYSFS_FCHOST, fchost, fchost);
> >> -		rc = fcm_fcoe_if_action(path, "- - -");
> >> +		rc = fcm_fcoe_if_action(path, "- - -", NULL);
> >>  		break;
> >>  	case FCP_VLAN_DISC:
> >>  		FCM_LOG_DBG("OP: VLAN DISC %s\n", p->ifname);
> >> @@ -2667,12 +2700,6 @@ int main(int argc, char **argv)
> >>  	}
> >>  	fcm_pidfile_create();
> >>
> >> -	/* check fcoe module */
> >> -	if (fcoe_checkdir(SYSFS_FCOE)) {
> >> -		FCM_LOG_ERR(errno, "make sure FCoE driver module is
> >> loaded!");
> >> -		exit(1);
> >> -	}
> >> -
> >>  	fcm_fcoe_init();
> >>  	fcm_link_init();	/* NETLINK_ROUTE protocol */
> >>  	fcm_dcbd_init();
> >> diff --git a/include/fcoe_utils.h b/include/fcoe_utils.h
> >> index 3c43304..4c9715d 100644
> >> --- a/include/fcoe_utils.h
> >> +++ b/include/fcoe_utils.h
> >> @@ -42,11 +42,13 @@
> >>
> >>  #define MAX_STR_LEN 512
> >>  #define MAX_PATH_LEN MAX_STR_LEN
> >> +#define MAX_DRV_NAME_LEN 32
> >> +
> >>
> >>  #define SYSFS_MOUNT	"/sys"
> >>  #define SYSFS_NET	SYSFS_MOUNT "/class/net"
> >>  #define SYSFS_FCHOST	SYSFS_MOUNT "/class/fc_host"
> >> -#define SYSFS_FCOE	SYSFS_MOUNT "/module/fcoe/parameters"
> >> +#define SYSFS_FCOE	SYSFS_MOUNT "/module/libfcoe/parameters"
> >>
> > Rob,
> > This change will impact existing fcoe-utils and may need some
> coordination
> > to make sure the kernel side goes in first, I guess it’s an overkill to
> add
> > kernel version tracking then build SYSFS_FCOE accordingly, might be
> simpler
> > just prompt the user to upgrade the fcoe-util.
> >
> > thanks,
> > yi
> >
> >>  #define FCHOSTBUFLEN 64
> >>
> >> --
> >> 1.7.1
> >>
> >>
> >>
> >>
> >> _______________________________________________
> >> 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