On Thu, 2011-01-06 at 13:52 -0800, Zou, Yi wrote: > > > > 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 for answering this Yi. The threads pointed by Yi have more detailed information. Thanks, Bhanu > > thanks, > > yi > > > > > > >> > > >> Signed-off-by: Nithin Nayak Sujir <[email protected]> > > >> --- > > >> 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 > > >> [email protected] > > >> https://lists.open-fcoe.org/mailman/listinfo/devel > > > _______________________________________________ > > > devel mailing list > > > [email protected] > > > https://lists.open-fcoe.org/mailman/listinfo/devel > _______________________________________________ devel mailing list [email protected] https://lists.open-fcoe.org/mailman/listinfo/devel
