[dpdk-dev] [PATCH v1 03/28] eal/linux: extract function rte_eal_unbind_kernel_driver

Jan Viktorin viktorin at rehivetech.com
Tue May 17 20:14:24 CEST 2016


On Fri, 13 May 2016 09:22:23 +0800
Jianbo Liu <jianbo.liu at linaro.org> wrote:

> On 6 May 2016 at 21:47, Jan Viktorin <viktorin at rehivetech.com> wrote:
> > Generalize the PCI-specific pci_unbind_kernel_driver. It is now divided into
> > two parts. First, determination of the path and string identification of the
> > device to be unbound. Second, the actual unbind operation which is generic.
> >
> > Signed-off-by: Jan Viktorin <viktorin at rehivetech.com>
> > ---
> >  lib/librte_eal/common/eal_private.h   | 13 +++++++++++++
> >  lib/librte_eal/linuxapp/eal/eal.c     | 26 ++++++++++++++++++++++++++
> >  lib/librte_eal/linuxapp/eal/eal_pci.c | 33 +++++++++------------------------
> >  3 files changed, 48 insertions(+), 24 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> > index 81816a6..3fb8353 100644
> > --- a/lib/librte_eal/common/eal_private.h
> > +++ b/lib/librte_eal/common/eal_private.h
> > @@ -289,6 +289,19 @@ int rte_eal_alarm_init(void);
> >  int rte_eal_check_module(const char *module_name);
> >
> >  /**
> > + * Unbind kernel driver bound to the device specified by the given devpath,
> > + * and its string identification.
> > + *
> > + * @param devpath  path to the device directory ("/sys/.../devices/<name>")
> > + * @param devid    identification of the device (<name>)
> > + *
> > + * @return
> > + *      -1  unbind has failed
> > + *       0  module has been unbound
> > + */
> > +int rte_eal_unbind_kernel_driver(const char *devpath, const char *devid);
> > +
> > +/**
> >   * Get cpu core_id.
> >   *
> >   * This function is private to the EAL.
> > diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> > index e8fce6b..844f958 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal.c
> > @@ -949,3 +949,29 @@ rte_eal_check_module(const char *module_name)
> >         /* Module has been found */
> >         return 1;
> >  }
> > +
> > +int
> > +rte_eal_unbind_kernel_driver(const char *devpath, const char *devid)
> > +{
> > +       char filename[PATH_MAX];
> > +       FILE *f;
> > +
> > +       snprintf(filename, sizeof(filename),
> > +                "%s/driver/unbind", devpath);
> > +
> > +       f = fopen(filename, "w");
> > +       if (f == NULL) /* device was not bound */
> > +               return 0;
> > +
> > +       if (fwrite(devid, strlen(devid), 1, f) == 0) {
> > +               RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__,
> > +                               filename);
> > +               goto error;
> > +       }
> > +
> > +       fclose(f);
> > +       return 0;
> > +error:
> > +       fclose(f);
> > +       return -1;
> > +}
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> > index fd7e34f..312cb14 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> > @@ -59,38 +59,23 @@ int
> >  pci_unbind_kernel_driver(struct rte_pci_device *dev)
> >  {
> >         int n;
> > -       FILE *f;
> > -       char filename[PATH_MAX];
> > -       char buf[BUFSIZ];
> > +       char devpath[PATH_MAX];
> > +       char devid[BUFSIZ];
> >         struct rte_pci_addr *loc = &dev->addr;
> >
> > -       /* open /sys/bus/pci/devices/AAAA:BB:CC.D/driver */
> > -       snprintf(filename, sizeof(filename),
> > -                SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver/unbind",
> > +       /* devpath /sys/bus/pci/devices/AAAA:BB:CC.D */
> > +       snprintf(devpath, sizeof(devpath),
> > +                SYSFS_PCI_DEVICES "/" PCI_PRI_FMT,
> >                  loc->domain, loc->bus, loc->devid, loc->function);
> >
> > -       f = fopen(filename, "w");
> > -       if (f == NULL) /* device was not bound */
> > -               return 0;
> > -
> > -       n = snprintf(buf, sizeof(buf), PCI_PRI_FMT "\n",
> > +       n = snprintf(devid, sizeof(devid), PCI_PRI_FMT "\n",
> >                      loc->domain, loc->bus, loc->devid, loc->function);
> > -       if ((n < 0) || (n >= (int)sizeof(buf))) {
> > +       if ((n < 0) || (n >= (int)sizeof(devid))) {  
> 
> Is it better to move "(n >= (int)sizeof(devid))" before snprintf and
> it has different reason from "n < 0"?

I don't understant this comment. I cannot move the check for _n_ before
the snprintf as it is its return value... Can you provide an example of
your idea?

Do you mean to split the condition to if (n < 0) and else if (n >= ...)?

> 
> >                 RTE_LOG(ERR, EAL, "%s(): snprintf failed\n", __func__);
> > -               goto error;
> > -       }
> > -       if (fwrite(buf, n, 1, f) == 0) {
> > -               RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__,
> > -                               filename);
> > -               goto error;
> > +               return -1;
> >         }
> >
> > -       fclose(f);
> > -       return 0;
> > -
> > -error:
> > -       fclose(f);
> > -       return -1;
> > +       return rte_eal_unbind_kernel_driver(devpath, devid);
> >  }
> >
> >  static int
> > --
> > 2.8.0
> >  



-- 
   Jan Viktorin                  E-mail: Viktorin at RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic


More information about the dev mailing list