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

Bhanu Gollapudi bprakash at broadcom.com
Thu Jan 6 22:34:16 UTC 2011


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 <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